Hello, Martin,

On Jun  2, 2020, Martin Liška <mli...@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.

-- 
Alexandre Oliva, freedom fighter    he/him    https://FSFLA.org/blogs/lxo/
Free Software Evangelist              Stallman was right, but he's left :(
GNU Toolchain Engineer           Live long and free, and prosper ethically

Reply via email to