[Patch] WinCOFFObjectWriter: fix for storing pointer to string table in header name field

Hi,

Recently I was hit by an assert in WinCOFFObjectWriter that had forbidden storing pointer to string table in header name field when the pointer had more that 6 decimal digits. This limit had been chosen to make implementation easier (sprintf adds null character at the end) and could be increased to 7 digits.

My patch is attached. The implementation uses additional buffer on the stack to make integer to string conversion.

  • Paweł Bylica

WinCOFFObjectWriter_PointerToStingTable.patch (1.49 KB)

I've already implemented this (and also included the undocumented base64 encoding), see: http://llvm-reviews.chandlerc.com/D667

Maybe someone actually hitting this limit in practice will make someone accept at least the first part of D667.

-Nico

Is there a problem if the string is not null terminated? If not, you can snprintf it right into place instead of doing sprintf+mempcy.

snprintf always null-terminates (and truncates if there's not enough space).

-Nico

On 23.07.2013 18:48, Nico Rieck wrote:> On 23.07.2013 18:43, Reid Kleckner wrote:
>> Is there a problem if the string is not null terminated? If not, you can
>> snprintf it right into place instead of doing sprintf+mempcy.
>
> snprintf always null-terminates (and truncates if there's not enough
> space).

Urgh, nevermind. Brain fart here.

-Nico

Nuh uh: "The _snprintf function formats and stores count or fewer
characters in buffer, and appends a terminating null character if the
formatted string length is strictly less than count characters."

Please don't assume snprintf always null terminates.

This may be Windows-specific behavior that you shouldn't rely on. If
that's the case, ignore my suggestion.

I'm reading C99 7.19.6.4 and C11 7.21.6.5 which says:

"If n is zero, nothing is written, and s may be a null pointer. Otherwise, output characters beyond the n-1st are discarded rather than being written to the array, and a null character is written at the end of the characters actually written into the array."

and "Thus, the null-terminated output has been
completely written if and only if the returned value is nonnegative and less than n."

-Nico

In article <CACs=ty+7zKZU6Ad4jZ5J5Rb0qoMa-bNtO0f+xK_8SYfy3RyFbg@mail.gmail.com>,
    Reid Kleckner <rnk@google.com> writes:

Is there a problem if the string is not null terminated? If not, you can
snprintf it right into place instead of doing sprintf+mempcy.

Am I the only one who scratches my head and says:

  sprintf?
  memcpy?

Why are we using error-prone C APIs in C++ code?

In article <CACs=tyLqAn7GNNobgwr+K96k-Zw1y5N16dzOeOB2O8OtrHyNig@mail.gmail.com>,
    Reid Kleckner <rnk@google.com> writes:

Nuh uh: "The _snprintf function formats and stores count or fewer
characters in buffer, and appends a terminating null character if the
formatted string length is strictly less than count characters."
_snprintf, _snprintf_l, _snwprintf, _snwprintf_l | Microsoft Docs

Please don't assume snprintf always null terminates.

This may be Windows-specific behavior that you shouldn't rely on. If
that's the case, ignore my suggestion.

Yes, _snprintf with MSVC is not a conforming C99 (or whatever) standard
implementation of snprintf. I've filed a bug on it, but don't expect
a fix from them anytime soon as they deem it low priority.

This is just another reason to stay away from these error-prone C APIs
in C++ code.

I've just kept the style of the previous implementation. I'd love to use
std::to_string(), but it is just not available. Moreover, you still have to
move this 8 bytes of text somehow.