[PATCH 1/9] Do not pass NULL to memcpy

[Simon Marchi]

On 2018-08-27 10:56 AM, Tom Tromey wrote:
> -fsanitize=undefined pointed out a couple of spots that pass NULL to
> memcpy, which is undefined behavior according to the C standard.
> 
> ChangeLog
> 2018-08-27  Tom Tromey  <tom@tromey.com>
> 
> 	* namespace.c (add_using_directive): Don't pass NULL to memcpy.
> 	* dwarf2-frame.h (struct dwarf2_frame_state_reg_info): Don't pass
> 	NULL to memcpy.
> ---
>  gdb/ChangeLog      | 6 ++++++
>  gdb/dwarf2-frame.h | 3 ++-
>  gdb/namespace.c    | 5 +++--
>  3 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h
> index 52316e5e168..6844010c8df 100644
> --- a/gdb/dwarf2-frame.h
> +++ b/gdb/dwarf2-frame.h
> @@ -110,7 +110,8 @@ struct dwarf2_frame_state_reg_info
>      size_t size = src.num_regs * sizeof (struct dwarf2_frame_state_reg);
>  
>      reg = (struct dwarf2_frame_state_reg *) xmalloc (size);
> -    memcpy (reg, src.reg, size);
> +    if (size > 0)
> +      memcpy (reg, src.reg, size);
>    }

While the patch does not look wrong, I think the problem would "solve itself"
"reg" was an std::vector, since an std::vector already has an appropriate copy
constructor.  It would also simplify a lot this area of the code, for example,
the alloc_regs method would become a one-liner.

>  
>    /* Assignment operator for both move-assignment and copy-assignment.  */
> diff --git a/gdb/namespace.c b/gdb/namespace.c
> index be998d9d491..85c0c4b14d7 100644
> --- a/gdb/namespace.c
> +++ b/gdb/namespace.c
> @@ -111,8 +111,9 @@ add_using_directive (struct using_direct **using_directives,
>    else
>      newobj->declaration = declaration;
>  
> -  memcpy (newobj->excludes, excludes.data (),
> -	  excludes.size () * sizeof (*newobj->excludes));
> +  if (!excludes.empty ())
> +    memcpy (newobj->excludes, excludes.data (),
> +	    excludes.size () * sizeof (*newobj->excludes));
>    newobj->excludes[excludes.size ()] = NULL;
>  
>    newobj->next = *using_directives;
> 

Here too it would be nice to have make using_direct::excludes an std::vector,
but it is not as simple, so I think adding that "if" is reasonnable.

Simon