[1/9] Do not pass NULL to memcpy
Commit Message
-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(-)
Comments
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
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
Simon> While the patch does not look wrong, I think the problem would "solve itself"
Simon> "reg" was an std::vector, since an std::vector already has an appropriate copy
Simon> constructor. It would also simplify a lot this area of the code, for example,
Simon> the alloc_regs method would become a one-liner.
I did this and I'll include it as a separate patch in the next round.
Tom
@@ -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);
}
/* Assignment operator for both move-assignment and copy-assignment. */
@@ -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;