[llvm-bugs] [Bug 43284] New: Memory corruption in frontend PGO pipeline

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=43284

            Bug ID: 43284
           Summary: Memory corruption in frontend PGO pipeline
           Product: clang
           Version: unspecified
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: LLVM Codegen
          Assignee: unassignedclangbugs at nondot.org
          Reporter: pk1574 at gmail.com
                CC: davidxl at google.com, llvm-bugs at lists.llvm.org,
                    neeilans at live.com, phosek at chromium.org,
                    richard-llvm at metafoo.co.uk, rnk at google.com,
                    vsk at apple.com

Created attachment 22486
  --> https://bugs.llvm.org/attachment.cgi?id=22486&action=edit
A git patch that can reproduce memory corruption in clang profile tests

When reading in profile data I noticed that it is possible for a function to be
modified from the original collected profile, but to still hash the same. I
initially found this when submitting D66324, and the ASAN bot broke on one of
my tests.

I used the same profile for two tests that were largely the same. They differed
in whether the expected value passed to __builtin_expect() was constant or not.
The functions here hash exactly the same, but require different numbers of
profile counters. During compilation, when profile weights are being added it
is possible for a mismatch to occur between the number of profile counters the
frontend tries to instantiate, and the number read in from the profile. 

The weights read in from the profile directly populate a vector during clang
code gen. The frontend also walks the AST and maps the statements it finds that
need a profiling counter to their expected index in the vector. When that index
is used, an out of bounds access may occur if there should be more profile
counters than are provided by the profile.

This can be reproduced simply in clang's profile tests, and I've included a git
patch file with the relevant change. When run under ASAN the
misexpect-switch-nonconst.c test should trigger the described memory
corruption.

The memory corruption itself happens in CodeGenPGO.h:112, but its root seems to
be a lack of warning about this mismatch when reading in the profile. Also
relevant is where the AST statement -> Counter mapping occurs in
CodeGenPGO.cpp:198, since this is where the invalid index is created.

My feeling is that either the hashed identifier needs to be more sensitive to
these changes (maybe by including the number of counters) which should avoid
the problem altogether, or that there needs to be a change to the clang/llvm
implementations to avoid the memory corruption.

Background on profile generation:
misexpect-switch.c was originally used to generate
Inputs/misexpect-switch.proftext. However, the profiled function hashes the
same in both Inputs/misexpect-switch.proftext and
Inputs/misexpect-switch-nonconst.proftext(generated from
misexpect-switch-nonconst.c), and causes an issues when
Inputs/misexpect-switch.proftext is used to compile misexpect-switch-nonconst.c
with PGO.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20190911/235bc0a9/attachment.html>