Re: Commit: Fix potential null pointer dereference in linker

[Alan Modra <amodra at gmail dot com>]

On Thu, Nov 21, 2019 at 05:02:10PM +0000, Nick Clifton wrote:
> Hi Guys,
> 
>   I am applying the patch below to fix a potential null pointer
>   dereference in the linker.

Actually, &file_chain.head->input_statement is exactly the same as
(lang_input_statement_type *) file_chain.head

Neither expression dereferences a pointer.  It's true in the general
case that the first expression might offset a pointer, and that
could require an extra check for NULL, but here the field offset is
zero.

>  This also fixes an annoying message from
>   the undefined behaviour sanitizer...

Yeah, well, nothing's perfect I guess.  I know in the past we've caved
in to every annoying false positive warning message from gcc, but if
we hide this sort of problem *in ubsan* will ubsan ever get fixed?

(The C standard does explicity say that in "&*p" the "*" is not
evaluated but unfortunately doesn't say the same for "->" in
"&p->field".  Which means ubsan authors need to use some common
sense.)

Is the ubsan complaint on every use of LANG_FOR_EACH_INPUT_STATEMENT,
and do you also get a complaint in ldlang.c:lang_for_each_input_file?
If not, then maybe something else is going on here.

See also PR23772, a related asan complaint that I'm about to close as
commit 36983a93bb avoided the construct.

> +  for (statement = file_chain.head == NULL ? NULL : &file_chain.head->input_statement; \

Please, can you just go back to using the cast here instead?  A while
ago I spent quite a bit of effort removing casts in ldlang.* but I
reckon a cast looks better than this overlong line.

-- 
Alan Modra
Australia Development Lab, IBM