Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overrun of data causes unassigned variable to be provided to functions #60

Open
Arrowana opened this issue Jun 28, 2023 · 12 comments
Open

Comments

@Arrowana
Copy link

Could also be broken with all similar functions but logging what I observed:

struct Option<T> {
    bool is_some;
    T value;
} [[format("format_option")]];

fn format_option(Option<auto> option) {
    std::print("{}", option.is_some);
    if (option.is_some) {
        return std::format("Some({})", option.value);
    } else {
        return "None";
    }
};

I can see in pattern data that is_some is true but inside format_option it is false (in the console). Value is also not populated.

@paxcut
Copy link
Contributor

paxcut commented Jun 28, 2023

you are not placing any variables in data. How is anything being printed in console other than this?

I: Pattern exited with code: 0
I: Evaluation took 0.0014823s

please provide input data too.

@paxcut
Copy link
Contributor

paxcut commented Jun 28, 2023

It seems to work fine but you are defining the template argument differently in function. using auto inside template creates non type template parameter, but original the template was defined as type parameter. Instead make the Option variable auto as in the code below.

I ran it on file with 01 on first and second value and it prints true for both the console and the pattern data

#include <type/guid.pat>


struct Option<T> {
    bool is_some;
    T value;
} [[format("format_option")]];

fn format_option(auto option) {
    std::print("{}", option.is_some);
    if (option.is_some) {
        return std::format("Some({})", option.value);
    } else {
        return "None";
    }
};


Option<u8> option @ 0;

image

PS: Don't call templates Generics. The documentation clearly uses the word template as the name for this feature. Generics is the name that C# uses instead of templates when they introduce the feature even though they use the keyword template in the feature itself.

@Arrowana
Copy link
Author

Arrowana commented Jun 28, 2023

It seems to work fine but you are defining the template argument differently in function. using auto inside template creates non type template parameter, but original the template was defined as type parameter. Instead make the Option variable auto as in the code below.

I ran it on file with 01 on first and second value and it prints true for both the console and the pattern data

#include <type/guid.pat>


struct Option<T> {
    bool is_some;
    T value;
} [[format("format_option")]];

fn format_option(auto option) {
    std::print("{}", option.is_some);
    if (option.is_some) {
        return std::format("Some({})", option.value);
    } else {
        return "None";
    }
};


Option<u8> option @ 0;

image

PS: Don't call templates Generics. The documentation clearly uses the word template as the name for this feature. Generics is the name that C# uses instead of templates when they introduce the feature even though they use the keyword template in the feature itself.

So i found what is the issue, my data was 64 bytes long but I was trying to parse 65 bytes total 😑. Looks like it silently breaks down, ending up with an empty argument in that function while the data inspector is fine and shows 0 as the last byte

struct Pubkey {
    u8 bytes[32];
};

struct A {
    u8 a[32];
    Option<Pubkey> b;
};

Regardless i'll use auto option.

To reproduce:
Create test data with pyhton, then first byte of Option can be toggled on and off

with open('test2.bin', 'wb+') as f:
    f.write(bytearray([i for i in range(32)] * 2))

Then the struct in format_option doesn't seem to contain anything, changing the first field to u8 a[31], it will start working as expected.

So i tried with something more basic

struct A {
    u8 a[32];
    u8 b[33] [[format("format_array")]];
};

fn format_array(auto array) {
    std::print("{}", array[0]);
    return "";
};

Array in format_array is empty when b overruns the data.

Thanks for the reply,

PS: Understood, i'll say template.

@paxcut
Copy link
Contributor

paxcut commented Jun 29, 2023

It doesn't silently break. The pattern will read data until it reaches the end of the file and then stop.
your data is

0,1,2, ... ,30,31,0,1,2, ... ,30,31

in the first case you assign 32 bytes to a[32] so

Option<Pubkey> option;

sees this data

0,1,2,   ...  ,30,31

then bool is_some becomes false (0) and there are 31 bytes left. so

T value;

becomes a

u8 bytes[32];

the first 31 bytes get values from file and the last one remains zero because thats the default value all of them have before assignment. The reason it appears not to read the 31 bytes is that

 bool is_some;

is false so format function returns "None", but if you open the variable for value you'll see all the values from 1 to 31 are assigned correctly.
On the second run you read 31 bytes into a[31] so

Option<Pubkey> option;

sees this data

31,0,1,2,   ...  ,30,31

now there are 33 bytes left and the first one is not zero so is_some now is true and the 32 bytes of the array get values from the file (the first one is still zero).

That explains the whole situation. When you were using

Option<auto> option

the error of using the wrong type means that option is not loaded with the value it had before calling format function so its values are all zero including the value of is_some is false.
This issue reports a problem using templates with [[format]] which does not appear to exist as long as you use the correct syntax when defining the format function. If you agree with that statement you should close this issue as resolved and if you have other
issues open another that explains what the issue is. If you still believe there is a problem with [[format]] and templates the best way to get help is to provide a minimal pattern that exhibits the error an input file or the means to generate one and instructions on how the error can be reproduced using them.

@paxcut
Copy link
Contributor

paxcut commented Jun 29, 2023

in this case

struct A {
    u8 a[32];
    u8 b[33] [[format("format_array")]];
};

fn format_array(auto array) {
    std::print("{}", array[0]);
    return "";
};

the problem is that your format function returns the empty string. what gets printed as value when you use a format function it the value of the string returned by the format function. using std::print inside the format function prints the data to the console which in this case is the first value of the array only which happens to be zero. it is not legal to pass arrays as arguments to functions. always create a struct around them and pass the struct instead.

ps: you can change the title of the issue at any time. please change Generics to Templates

@Arrowana
Copy link
Author

Arrowana commented Jun 29, 2023

Sorry, it was indeed confusing, I did say i mutated the data but not clearly at which stage.

option.hexpat

#include <std/io.pat>

struct Option<T> {
    bool is_some;
    T value;
} [[format("format_option")]];

fn format_option(auto option) {
    std::print("{}", option.is_some);
    if (option.is_some) {
        return std::format("Some({})", option.value);
    } else {
        return "None";
    }
};

struct A {
    u8 first;
    Option<u8> second;
};

A data @ 0x00;

Binary file test-case.bin (this seems easier that trying to share a binary file here).

with open('test-case.bin', 'wb+') as f:
       f.write(b'\x01\x02')

Expected:
An error since second overruns the data, which leads to format_option having some odd behaviour

Observed:
console prints false, while second.is_some is true in the pattern data.

Screenshot 2023-06-29 at 10 57 07 am

I hope it makes sense now

@paxcut
Copy link
Contributor

paxcut commented Jun 29, 2023

If there is an error in imhex that your code is showing then the error has nothing to do with templates and formats. to see that then the error should disappear when templates are not used but if you use

#include <std/io.pat>

struct Option {
    bool is_some;
    u8 value;
} [[format("format_option")]];

fn format_option(Option option) {
    std::print("{}", option.is_some);
    if (option.is_some) {
        return std::format("Some({})", option.value);
    } else {
        return "None";
    }
};

struct A {
    u8 first;
    Option second;
};

A data @ 0x00;

nothing changes in the output so templates are not to blame. As for whether or not the extra byte that the pattern has should cause an overrun I am not sure if that is a bug. if you add a second byte to value

u8 value[2];

you will see the error about going past the end. The pattern may allow a non existing byte of value zero at the end of the file for some reason.

@Arrowana
Copy link
Author

"If there is an error in imhex that your code is showing then the error has nothing to do with templates and formats. to see that then the error should disappear when templates are not used but if you use"
Yes, that's correct, but still a bug.

"you will see the error about going past the end. The pattern may allow a non existing byte of value zero at the end of the file for some reason." yes it does that but in addition silently allows format to run on the "empty" argument.

@paxcut
Copy link
Contributor

paxcut commented Jun 29, 2023

you need to change the title of the issue to reflect the true nature of the error you are seeing or close it and open a new one

@paxcut
Copy link
Contributor

paxcut commented Jun 29, 2023

It is definitely does not silently allow it to run.

  1. there is no empty argument. there is a variable that is not assigned a value so it remains as zero.
  2. is_some is not zero, but the print statement prints false and the value of the Option variable is reported as "None" and not as "Some(0)" as you would expect if it was running normally with the zero in value. The fact that it is reporting different values for the same variable means that there is an error in the pattern that needs to be fixed. It may not explicitly convey the nature of the error nut imhex hardly ever does that. Please change he title so that other uses don't use your example as a confirmation that templates don't work with [[format]]. Changing the title is very easy. scroll to the top of this page and next to the title you'll see an edit button. Click it and edit the title and click to confirm the change. the old title is not discarded and it will show that you changed the title for the old one to the new one. This is very important to ensure that people are looking at real errors and don't waste time trying to fix non-existing bugs. Since you are the only one that can edit it we count on you to make the change. you may choose not to and not provide explanations for it but that will reflect negatively on the impression you give to everyone.

@Arrowana Arrowana changed the title Generics don't seem to work properly with [[format("...")]] Overrun of data causes unassigned variable to be provided to functions Jun 29, 2023
@Arrowana
Copy link
Author

  1. 100% agree, is the new title appropriate?

@paxcut
Copy link
Contributor

paxcut commented Jun 29, 2023

if that was true then making the file size 3 and leaving last value of zero should have no effect on the output but thats not true. the value for the bool in the console changes and the value printed for Option variable also goes from "None" to "Some(0)". it still is a vast improvement over the previous title. thank you for doing that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants