[Compiler-rt] Fix InstrProfilingFile writing on Windows

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 14 15:46:54 PST 2015


On Mon, Dec 14, 2015 at 3:37 PM, Johan Engelen <jbc.engelen at gmail.com> wrote:
> Hi David,
>
> On Tue, Dec 15, 2015 at 12:06 AM, Xinliang David Li <davidxl at google.com>
> wrote:
>>
>> For the second one, I suggest adding one portability macro for the
>> mode strings in InstrProfilingPort.h.
>
>
> Because the instrprof file is a binary file, isn't it better on any platform
> to fopen with mode string "ab"?

Yes -- in theory ;) Ok with this change.

thanks,

David

> I don't see the benefit of making the mode-string "portable", but perhaps I
> am missing something.
>
> regards,
>   Johan
>
>
> On Tue, Dec 15, 2015 at 12:06 AM, Xinliang David Li <davidxl at google.com>
> wrote:
>>
>> Can you submit both fixes to trunk?
>>
>> For the second one, I suggest adding one portability macro for the
>> mode strings in InstrProfilingPort.h.
>>
>> thanks,
>>
>> David
>>
>> On Mon, Dec 14, 2015 at 3:01 PM, Johan Engelen <jbc.engelen at gmail.com>
>> wrote:
>> > Hello,
>> >   I recently started using compiler-rt/lib/profile for
>> > Instrumentation-based
>> > PGO in LDC (LLVM D Compiler), but ran into two issues with the code on
>> > Windows:
>> > (1) The code does not compile with MSVC because of "__attribute__"
>> > (2) The file written by InstrProfilingFile.c is in a corrupt format on
>> > Windows
>> >
>> >
>> > (1) is easily fixed, and I will submit a patch in a second email.
>> >
>> >
>> > (2) The file is corrupt because the file is opened in text mode with
>> > fopen(..., "a"), instead of in binary mode fopen(..., "ab"). With MSVC,
>> > this
>> > means that fwrite will silently add bytes to "fix" line-endings (i.e.
>> > 0x0A
>> > --> 0x0D 0x0A) !
>> > The bug fix is trivial: open the file explicitly in binary mode.
>> >
>> > I have attached a patch for the release_37 branch (I much appreciate it
>> > if
>> > it can be committed, because I currently use this version of
>> > /lib/profile
>> > successfully for PGO in LDC), and I have also attached a patch for
>> > trunk.
>> >
>> > (btw, thanks for the work on /lib/profile! It made it very easy to add
>> > PGO
>> > to LDC and worked flawlessly on OS X and Ubuntu!)
>> >
>> > Cheers,
>> >   Johan
>> >
>
>


More information about the llvm-commits mailing list