parse_floats invoked in a way that induces undefined behaviour

[dneto0]

The parse_floats function in vr-script is at https://github.com/Igalia/vkrunner/blob/master/vkrunner/vr-script.c#L904

It has signature

static bool
parse_floats(const char **p,
             float *out,
             int n_floats,
             const char *sep)

The intent is to parse n_floats and write the results to an array of floats rooted at out.

For example, it is used to parse 4 floats of a draw_rect command, at https://github.com/Igalia/vkrunner/blob/master/vkrunner/vr-script.c#L904

That call is:

        if (!parse_floats(&p, &command->draw_rect.x, 4, NULL) 

Where it's referencing the x element of the following struct:

                struct {
                        float x, y, w, h;
                        unsigned pipeline_key;
                } draw_rect;

The intent is to fill the x, y, w, h components. But as parse_floats executes, it walks past the end of the x element and that induces undefined behaviour.

See C11 "6.5.6 Additive operators". Paragraph 7 says that if you have a pointer to something that isn't an array, then treat it as if it's a pointer to an array of length 1. Paragraph 9 talks about pointer addition and subtraction, and the treatement of pointers one-past the end of an array. The last statement is:

If the result points one past the last element of the array object, it
shall not be used as the operand of a unary * operator that is evaluated.

But that's what happens when parse_floats increments the pointer beyond x to reach y.

The practical issue is that an implementation might insert padding between struct elements and you don't get what you expect.

[bpeel]

Is this a purely theoretical issue or is there really a compiler that would insert a different amount of padding between sequential struct members of the same type than it would between elements of an array of that type? If it’s not causing a real problem I’m inclined to just leave it as it is. I think this is quite a common thing to do in 3D programming. For example whenever something uses a struct Vertex with x, y and z members and then passes it to glVertexAttribPointer.

[dneto0]

Undefined behaviour is undefined behaviour. Newer compilers are getting very aggressive at doing very drastic changes to your code if they detect undefined behaviour.
Leaving this in will be a distraction any time someone else reviews the code for undef behaviour in the future.

[dneto0]

Thank you. :-)