[llvm] r217708 - llvm-profdata: Avoid undefined behaviour when reading raw profiles

[Justin Bogner]

David Blaikie <dblaikie at gmail.com> writes:
> On Fri, Sep 12, 2014 at 2:22 PM, Justin Bogner <mail at justinbogner.com> wrote:
>
>     Author: bogner
>     Date: Fri Sep 12 16:22:55 2014
>     New Revision: 217708
>    
>     URL: http://llvm.org/viewvc/llvm-project?rev=217708&view=rev
>     Log:
>     llvm-profdata: Avoid undefined behaviour when reading raw profiles
>    
>     The raw profiles that are generated in compiler-rt always add padding
>     so that each profile is aligned, so we can simply treat files that
>     don't have this property as malformed.
>
> Any chance of changing this to not rely on UB (this code looks like it's
> violating C++ aliasing rules pretty consistently) and instead using memcpy?

It's always casting from char *, which IIUC doesn't break aliasing
rules, so I *think* that this isn't relying on UB.

Assuming I'm right about that, the current design avoids unnecessarily
copying from the file to an auxiliary structure just to read the data
and throw it away.

>     Caught by Alexey's new ubsan bot. Thanks!
>    
>     Modified:
>         llvm/trunk/lib/ProfileData/InstrProfReader.cpp
>         llvm/trunk/test/tools/llvm-profdata/raw-two-profiles.test
>    
>     Modified: llvm/trunk/lib/ProfileData/InstrProfReader.cpp
>     URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/
>     InstrProfReader.cpp?rev=217708&r1=217707&r2=217708&view=diff
>     ==========================================================================
>     ====
>     --- llvm/trunk/lib/ProfileData/InstrProfReader.cpp (original)
>     +++ llvm/trunk/lib/ProfileData/InstrProfReader.cpp Fri Sep 12 16:22:55
>     2014
>     @@ -190,6 +190,9 @@ RawInstrProfReader<IntPtrT>::readNextHea
>        // garbage at the end of the file.
>        if (CurrentPos + sizeof(RawHeader) > End)
>          return instrprof_error::malformed;
>     +  // The writer ensures each profile is padded to start at an aligned
>     address.
>     +  if (reinterpret_cast<size_t>(CurrentPos) % alignOf<uint64_t>())
>     +    return instrprof_error::malformed;
>        // The magic should have the same byte order as in the previous header.
>        uint64_t Magic = *reinterpret_cast<const uint64_t *>(CurrentPos);
>        if (Magic != swap(getRawMagic<IntPtrT>()))
>    
>     Modified: llvm/trunk/test/tools/llvm-profdata/raw-two-profiles.test
>     URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/
>     llvm-profdata/raw-two-profiles.test?rev=217708&r1=217707&r2=217708&view=
>     diff
>     ==========================================================================
>     ====
>     --- llvm/trunk/test/tools/llvm-profdata/raw-two-profiles.test (original)
>     +++ llvm/trunk/test/tools/llvm-profdata/raw-two-profiles.test Fri Sep 12
>     16:22:55 2014
>     @@ -39,11 +39,9 @@ RUN: printf '\0\0\0\0\0' >> %t-foo-padde
>      RUN: cat %t-bar.profraw > %t-bar-padded.profraw
>      RUN: printf '\0\0\0\0\0' >> %t-bar-padded.profraw
>    
>     -RUN: cat %t-foo.profraw %t-bar.profraw > %t-nopad.profraw
>      RUN: cat %t-foo-padded.profraw %t-bar.profraw > %t-pad-between.profraw
>      RUN: cat %t-foo-padded.profraw %t-bar-padded.profraw > %t-pad.profraw
>    
>     -RUN: llvm-profdata show %t-nopad.profraw -all-functions -counts |
>     FileCheck %s
>      RUN: llvm-profdata show %t-pad-between.profraw -all-functions -counts |
>     FileCheck %s
>      RUN: llvm-profdata show %t-pad.profraw -all-functions -counts | FileCheck
>     %s
>
>     _______________________________________________
>     llvm-commits mailing list
>     llvm-commits at cs.uiuc.edu
>     http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits