[Feature] stop command

[kbock-dev]

What feature would you like to see?

Stop at any point in an ImHex pattern.

How will this feature be useful to you and others?

During development of a pattern I'd like to stop the pattern from executing further and keep the currently created pattern data. It might also useful for file types that will change in the future and unknown data is encountered then.

Request Type

• I can provide a PoC for this feature or am willing to work on it myself and submit a PR

Additional context?

I've mostly completed an implementation but I'm not sure if it would be merged since it can usually be achieved by something like

$ = std::mem::size();

A stop control flow command would make the intention clearer in the pattern and works everywhere (i.e. functions), but might clutter tests due to the nature of the command as each case needs it's own test and one can usually use a workaround so missing this feature is not that important.

[paxcut]

Why not use the return control flow statement? when placed in a struct makes the pattern stop evaluating when it is reached. It doesn't work like that inside functions, but patterns cannot be created inside functions so that functionality has little use.

Also you can place a breakpoint anywhere and run the pattern. The debugger tab will contain the patterns created so far.

[kbock-dev]

Why not use the return control flow statement? when placed in a struct makes the pattern stop evaluating when it is reached.

Return works in many cases but not directly in something like

struct Test {
    u32 test = std::mem::read_unsigned($, 4);
    match (test) {
        (<some value>): Test2 test2;
        (<other value>): Test3 test3;
        (_): return;
    }
};

struct File {
    ...
    Test test[while (!std::mem::eof())];
};

File file @ 0;

which would need to be refactored to work as intended.

Also you can place a breakpoint anywhere and run the pattern. The debugger tab will contain the patterns created so far.

Yes, a stop command would be slightly more convinient.

I know there are workarounds/other ways to do it that's why I'd like to inquire with this ticket if this still would be useful enough (to provide other options) or not.

[paxcut]

The posted code is incomplete and cannot be tested. There is no explanation as to why it doesn't work as expected and what refactoring would be needed to work. The posted link to the documentation of return describe it as exactly the feature you are requesting.

Return statements

Return statements outside of functions can be used to prematurely terminate execution of the current program.

Evaluation stops at the location the return statement was executed. All patterns that have been evaluated up until this point will be finished up and placed into memory before execution halts.

[kbock-dev]

It wasn't intended to be a complete example, I just thought it would illustrate the issue enough. Here is a complete/testable one (but still only for testing purposes, not a real world example).

import std.mem;

struct Test2 {
    u32 test;
};

struct Test3 {
    u32 test;
};

struct Test {
    u32 test = std::mem::read_unsigned($, 4);
    match (test) {
        (0): Test2 test2;
        (1): Test3 test3;
        (_): return;
    }
};

struct File {
    Test test[while (!std::mem::eof())];
};

File file @ 0;

If the file that it's used on is not too short (and doesn't just have zero or ones in the 32 bit values 😉), it loops until the loop limit is reached, return doesn't stop the evalution and the default case doesn't change anything at all, omitting it works the same. To prevent reaching the loop maximum, the while loop condition needs to be changed which would be unneccessary with a stop (here my previously mentioned workaround works).

[paxcut]

If the value read is not 0 or 1 then according to the documentation the pattern should stop evaluating. If it doesn't that means there is a bug which needs to be fixed.

[paxcut]

The bug was confirmed upon testing your example. The cause was the the return flow control command was not being checked for if a dynamic array has a while sized statement. Adding the check makes your test example run as expected without any modifications. The fix will be posted in a PR sometime soon.

Are there other cases where return doesn't work like it is described in the docs that you know of?

I'll post a note here when the code has been merged so you can test it.

[paxcut]

I created the pr (with the wrong name) and it was merged.It still needs to be updated in the imhex repo to become part of master but if you want to try the changes, here's the PR #229

[kbock-dev]

Issues I found so far:

• It breaks existing patterns, for example https://github.com/WerWolv/ImHex-Patterns/blob/master/patterns/stdfv4.hexpat stops prematurely now. Testfile can be found at https://roos.com/docs/RBEH-AH4R8T?Open Haven't checked more than this one. Several patterns might have to be adapted.
• return doesn't stop this

// data for pattern: 00 FF 02 03 FF 05 06 07 08 09 0A 0B 0C 0D 0E 0F

import std.mem;

struct Test2 {
    s8 test;
    if (test > 0)
        return;
    s8 test2;
};

struct Test {
    s8 test;
    Test2 test2[while(!std::mem::eof())];
};

struct File {
    Test test[while(!std::mem::eof())];
};

File file @ 0;

[paxcut]

It breaks existing patterns

What do you mean it stops prematurely? Before it wouldn't stop even if a return was executed inside a while sized array element. More likely before it was running too far incorrectly and now it stops correctly.
If patterns are written incorrectly because of a bug ten they will need to be fixed if the bug is fixed.

return doesn't stop this

Yes, the fix I created was resetting the control flow prematurely to none so a return found in nested while sized arrays would only stop the inner array, but to the outer array control flow was becoming none. I fixed it now so that it also works on the nested array example as well.

[paxcut]

changes can be found in the new pr #230 i just opened

[kbock-dev]

What do you mean it stops prematurely? Before it wouldn't stop even if a return was executed inside a while sized array element. More likely before it was running too far incorrectly and now it stops correctly.

Before it parsed the whole file (example from the link above) with 190723 bytes (exact file size), 6201 struct Records. Now it parses just 6264 bytes, 111 struct Records. It has a loop for the Records (Record records[while(!std::mem::eof())];) and uses several returns inside structs leading to a different result than it seems to intend. So it likely was running properly (but not adhering to specs then) and now it doesn't.

If patterns are written incorrectly because of a bug ten they will need to be fixed if the bug is fixed.

That's true. Just wanted to raise awareness that there might be some patterns that don't work properly anymore until they are inspected and fixed.

changes can be found in the new pr #230 i just opened

Thanks. I try to check later to see if I find more cases that don't work yet.

[paxcut]

It has a loop for the Records (Record records[while(!std::mem::eof())];) and uses several returns inside structs leading to a different result than it seems to intend. So it likely was running properly (but not adhering to specs then) and now it doesn't.

I havent had a chance to test it yet so I can't really say where the problem lies.

Just wanted to raise awareness that there might be some patterns that don't work properly anymore until they are inspected and fixed.

But that's incorrect. If one assumes that the return statement is working correctly and there are patterns that with the correct implementation no longer give the same result as before then the only logical conclusion is that the patterns didn't work properly to begin with and their erroneous behavior was being masked by the bug in return. An improperly written pattern needs to be fixed regardless of any other consideration and one should never refrain from fixing a bug so that invalid code can continue to run as if it was valid due to the bug.

Thanks. I try to check later to see if I find more cases that don't work yet.

I have gone through all the code in the pattern language that uses flow control statements to verify that is does what it is supposed to so now I am more confident in asserting that you probably won't be able to find any such case. Of course nothing is absolute and I appreciate the help in finding the problems we have found so far.

[paxcut]

Ok, I had a chance to test the file that you said should be read entirely but stops at the 111 records. The last record is a PTR as shown

As you can see the last value in the record is ALARM_ID so for reference lets see what a PTR looks like from its start to ALARM_ID

struct PTR : Header {
    u64 start = $;
    u32 TEST_NUM [[comment("Test number")]];
    u8 HEAD_NUM [[comment("Test head number")]];
    u8 SITE_NUM [[comment("Test site number")]];
    PTRTestFlg TEST_FLG [[comment("Test flags (fail, alarm, etc.)")]];
    ParamFlg PARM_FLG [[comment("Parametric test flags (drift, etc.)")]];
    if (($ - start) == REC_LEN) { return; }
    float RESULT [[comment("Test result (only valid if TEST_FLG bit 1 = 1)")]];
    if (($ - start) == REC_LEN) { return; }
    VarLenStr TEST_TXT [[comment("Test description text or label (absent if length byte = 0)")]];
    if (($ - start) == REC_LEN) { return; }
    VarLenStr ALARM_ID [[comment("Name of alarm (absent if length byte = 0)")]];
    if (($ - start) == REC_LEN) { return; }

so it looks like if after ALARM_ID $-start equals REC_LEN then the pattern should stop. $-start is just the size of the data stored in the pattern after start is loaded and $-start is evaluated, that is the size of the data stored starting at TEST_NUM and all the way to and including ALARM_ID. Seeing above that REC_LEN is 20 we add the sizes in the size column with TEST_NUM being 4.
that is 4+1+1+1+1+4+7+1 which is exactly equal to 20.

From looking at the code I think the intention is that REC_LEN represents the record's length. then the code
if (($ - start) == REC_LEN) { return; }
is meant to prevent the record from adding more data, except that's not what it should do. You created incorrect code that worked only because of a bug.
How should the pattern be written then? you can't use break because it will affect array so you need to do
if (($ - start) < REC_LEN) {
....
if (($ - start) < REC_LEN) {
....
}
}
which is a pain with all the large structs used but using a text editor with macros makes the task much easier and faster.
I made the change and it works as expected by parsing the entire file with the values you posted.

[kbock-dev]

You created incorrect code that worked only because of a bug.

This is a misunderstanding. I did not write this pattern nor do I have any interest in it at all. It was just a pattern that existed in the pattern repo that used returns in structs (randomly selected on this criteria) and where I checked if the changes affect existing patterns.

I have gone through all the code in the pattern language that uses flow control statements

I haven't found more failing test cases, so I'm closing this issue as return is indeed the intended solution and works now after bugfixing.