Re: [PATCH 1/2] gdb: check for empty offsets vector in inherit_abstract_dies

[Tom Tromey <tom@tromey.com>]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

>> This subsequent loop does stuff like:
>> 
>> while (offsetp < offsets_end
>> && *offsetp < origin_child_die->sect_off)
>> offsetp++;
>> 
>> which in this scenario would be comparing null pointers using "<"?

Simon> Is it undefined behavior to compre two null pointers (both offsetp and
Simon> offset_end would be nullptr or both wouldn't be nullptr)?  At least
Simon> clang's UB sanitizer doesn't panic.

I don't know but it's certainly weird.

>> I'm guessing this whole loop should be hoisted into the 'if'.

Simon> I don't understand enough what the code does to be convinced this would
Simon> be a correct change.

Any real work done in the loop is conditional on checking 'offsetp',
which will always be NULL in this scenario.

The loop also updates origin_child_die, but that's not used after the
loop completes.

So while I also think the code is super obscure, it also seems correct
to do this hoisting.

Simon> Note that the following patch changes this to use iterators, so it makes
Simon> the worry about comparing null pointers being undefined behavior go
Simon> away.  It becomes comparing offsets.end() with offsets.end().

Yeah.

Tom