Hello, Martin,
On Jun 2, 2020, Martin Liška <[email protected]> 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