Do not copy NULL string with memcpy.

The problem happens when we generate temp file
for .res file. Tested locally with the problematic
options.

Ready for master?
Thanks,
Martin

gcc/ChangeLog:

	PR driver/95456
	* gcc.c (do_spec_1): Append to tempfile only when
	input_basename != NULL.
---
  gcc/gcc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Hello, Martin,

On Jun  2, 2020, Martin Liška <mliska@suse.cz> wrote:

> The problem happens when we generate temp file
> for .res file. Tested locally with the problematic
> options.

Thanks for looking into this.

> Ready for master?

Erhm, no, I don't think that's correct.

With local analysis, the length computation just before has only
allocated space for basename_length.  If you copy outbase_length, you
might be writing past the end of the allocated area.


I guess basename_length is 0, otherwise you'd see a crash without the
assert you added in the PR.

With some global analysis (and running the testcase), it appears to me
that when input_basename is NULL (e.g., when processing linker specs),
so is outbase, so your proposed change appears to be trading one 0-byte
memcpy from NULL for another.

I'd rather make it:

  if (!outbase_length)
    {
      if (basename_length)
        memcpy (tmp + dumpdir_length, input_basename, basename_length);
    }
  else
    memcpy (tmp + dumpdir_length, outbase, outbase_length);

or maybe:

  if (outbase_length)
    memcpy (tmp + dumpdir_length, outbase, outbase_length);
  else if (basename_length)
    memcpy (tmp + dumpdir_length, input_basename, basename_length);


Please let me know if you'd prefer me to take this PR over.

On 6/3/20 5:58 AM, Alexandre Oliva wrote:
> Please let me know if you'd prefer me to take this PR over.

Yes, please take a look.

Martin

On Jun  3, 2020, Martin Liška <mliska@suse.cz> wrote:

> On 6/3/20 5:58 AM, Alexandre Oliva wrote:
>> Please let me know if you'd prefer me to take this PR over.

> Yes, please take a look.

Here's what I've regstrapped on x86_64-linux-gnu.  It makes both memcpy
calls conditional, and reorders the length computation to match.

I also ran outputs.exp with a memcpy wrapper in gcc.c to detect any
calls with NULL pointers, and nothing else came up.

Ok to install?


[PR95456] avoid memcpy (_, NULL, 0) in gcc.c

From: Alexandre Oliva <oliva@adacore.com>

Some newly-added code in gcc.c might call memcpy with a NULL source
pointer and zero-length inputs.  Avoid such calls by rearranging the
code a little.


for  gcc/ChangeLog

	PR driver/95456
	* gcc.c (do_spec_1): Don't call memcpy (_, NULL, 0).
---
 gcc/gcc.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/gcc/gcc.c b/gcc/gcc.c
index e2362175f4..c0eb3c1 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -6024,19 +6024,19 @@ do_spec_1 (const char *spec, int inswitch, const char *soft_matched_part)
 		      }
 		    temp_filename_length
 		      = dumpdir_length + suffix_length + 1;
-		    if (!outbase_length)
-		      temp_filename_length += basename_length;
-		    else
+		    if (outbase_length)
 		      temp_filename_length += outbase_length;
+		    else
+		      temp_filename_length += basename_length;
 		    tmp = (char *) alloca (temp_filename_length);
 		    if (dumpdir_length)
 		      memcpy (tmp, dumpdir, dumpdir_length);
-		    if (!outbase_length)
-		      memcpy (tmp + dumpdir_length, input_basename,
-			      basename_length);
-		    else
+		    if (outbase_length)
 		      memcpy (tmp + dumpdir_length, outbase,
 			      outbase_length);
+		    else if (basename_length)
+		      memcpy (tmp + dumpdir_length, input_basename,
+			      basename_length);
 		    memcpy (tmp + temp_filename_length - suffix_length - 1,
 			    suffix, suffix_length);
 		    if (adjusted_suffix)

On June 4, 2020 10:22:55 PM GMT+02:00, Alexandre Oliva <oliva@adacore.com> wrote:
>On Jun  3, 2020, Martin Liška <mliska@suse.cz> wrote:
>
>> On 6/3/20 5:58 AM, Alexandre Oliva wrote:
>>> Please let me know if you'd prefer me to take this PR over.
>
>> Yes, please take a look.
>
>Here's what I've regstrapped on x86_64-linux-gnu.  It makes both memcpy
>calls conditional, and reorders the length computation to match.
>
>I also ran outputs.exp with a memcpy wrapper in gcc.c to detect any
>calls with NULL pointers, and nothing else came up.
>
>Ok to install?

OK. 

Thanks, 
Richard. 

>
>[PR95456] avoid memcpy (_, NULL, 0) in gcc.c
>
>From: Alexandre Oliva <oliva@adacore.com>
>
>Some newly-added code in gcc.c might call memcpy with a NULL source
>pointer and zero-length inputs.  Avoid such calls by rearranging the
>code a little.
>
>
>for  gcc/ChangeLog
>
>	PR driver/95456
>	* gcc.c (do_spec_1): Don't call memcpy (_, NULL, 0).
>---
> gcc/gcc.c |   14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
>diff --git a/gcc/gcc.c b/gcc/gcc.c
>index e2362175f4..c0eb3c1 100644
>--- a/gcc/gcc.c
>+++ b/gcc/gcc.c
>@@ -6024,19 +6024,19 @@ do_spec_1 (const char *spec, int inswitch,
>const char *soft_matched_part)
> 		      }
> 		    temp_filename_length
> 		      = dumpdir_length + suffix_length + 1;
>-		    if (!outbase_length)
>-		      temp_filename_length += basename_length;
>-		    else
>+		    if (outbase_length)
> 		      temp_filename_length += outbase_length;
>+		    else
>+		      temp_filename_length += basename_length;
> 		    tmp = (char *) alloca (temp_filename_length);
> 		    if (dumpdir_length)
> 		      memcpy (tmp, dumpdir, dumpdir_length);
>-		    if (!outbase_length)
>-		      memcpy (tmp + dumpdir_length, input_basename,
>-			      basename_length);
>-		    else
>+		    if (outbase_length)
> 		      memcpy (tmp + dumpdir_length, outbase,
> 			      outbase_length);
>+		    else if (basename_length)
>+		      memcpy (tmp + dumpdir_length, input_basename,
>+			      basename_length);
> 		    memcpy (tmp + temp_filename_length - suffix_length - 1,
> 			    suffix, suffix_length);
> 		    if (adjusted_suffix)