[compiler-rt] r271349 - [profile] Fix PR/27917

[Xinliang David Li via llvm-commits]

The regression tests already covers -m32 -- see r261344.

The failure was not triggered unfortunately because the out of bound bytes
happen to be zero initialized.

David

On Wed, Jun 1, 2016 at 10:19 AM, Vedant Kumar <vsk at apple.com> wrote:

> I can see that it's tricky to check in a regression test for this.
>
> Would having a linux bot that tests the "x86-64 + -m32" configuration help?
>
> E.g:
>
> --- a/cmake/config-ix.cmake
> +++ b/cmake/config-ix.cmake
> @@ -93,13 +93,17 @@ set(COMPILER_RT_SUPPORTED_ARCH)
>  set(SIMPLE_SOURCE ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/simple.cc)
>  file(WRITE ${SIMPLE_SOURCE} "#include <stdlib.h>\n#include <limits>\nint
> main() {}\n")
>
> -# Add $arch as supported with no additional flags.
> -macro(add_default_target_arch arch)
> -  set(TARGET_${arch}_CFLAGS "")
> +# Add $arch as supported.
> +macro(add_default_target_arch arch cflags)
> +  set(TARGET_${arch}_CFLAGS ${cflags})
>    set(CAN_TARGET_${arch} 1)
>    list(APPEND COMPILER_RT_SUPPORTED_ARCH ${arch})
>  endmacro()
>
> +macro(add_default_target_arch arch)
> +  add_target_arch(arch "")
> +endmacro()
> +
>  macro(detect_target_arch)
>    check_symbol_exists(__arm__ "" __ARM)
>    check_symbol_exists(__aarch64__ "" __AARCH64)
> @@ -117,6 +121,7 @@ macro(detect_target_arch)
>      add_default_target_arch(aarch64)
>    elseif(__X86_64)
>      add_default_target_arch(x86_64)
> +    add_target_arch(x86_64_m32 "-m32")
>    elseif(__I686)
>      add_default_target_arch(i686)
>    elseif(__I386)
>
> vedant
>
> > On May 31, 2016, at 4:12 PM, Xinliang David Li via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> >
> > Author: davidxl
> > Date: Tue May 31 18:12:13 2016
> > New Revision: 271349
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=271349&view=rev
> > Log:
> > [profile] Fix PR/27917
> >
> > Skip the last (possibly) incomplete node from padding bytes.
> >
> >
> > Modified:
> >    compiler-rt/trunk/lib/profile/InstrProfilingValue.c
> >
> > Modified: compiler-rt/trunk/lib/profile/InstrProfilingValue.c
> > URL:
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/profile/InstrProfilingValue.c?rev=271349&r1=271348&r2=271349&view=diff
> >
> ==============================================================================
> > --- compiler-rt/trunk/lib/profile/InstrProfilingValue.c (original)
> > +++ compiler-rt/trunk/lib/profile/InstrProfilingValue.c Tue May 31
> 18:12:13 2016
> > @@ -111,7 +111,7 @@ static ValueProfNode *allocateOneNode(__
> >     return (ValueProfNode *)calloc(1, sizeof(ValueProfNode));
> >
> >   /* Early check to avoid value wrapping around.  */
> > -  if (CurrentVNode >= EndVNode) {
> > +  if (CurrentVNode + 1 > EndVNode) {
> >     if (OutOfNodesWarnings++ < INSTR_PROF_MAX_VP_WARNS) {
> >       PROF_WARN("Unable to track new values: %s. "
> >                 " Consider using option -mllvm -vp-counters-per-site=<n>
> to "
> > @@ -122,7 +122,9 @@ static ValueProfNode *allocateOneNode(__
> >     return 0;
> >   }
> >   Node = COMPILER_RT_PTR_FETCH_ADD(ValueProfNode, CurrentVNode, 1);
> > -  if (Node >= EndVNode)
> > +  /* Due to section padding, EndVNode point to a byte which is one pass
> > +   * an incomplete VNode, so we need to skip the last incomplete node.
> */
> > +  if (Node + 1 > EndVNode)
> >     return 0;
> >
> >   return Node;
> > @@ -201,7 +203,6 @@ __llvm_profile_instrument_target(uint64_
> >   CurVNode = allocateOneNode(PData, CounterIndex, TargetValue);
> >   if (!CurVNode)
> >     return;
> > -
> >   CurVNode->Value = TargetValue;
> >   CurVNode->Count++;
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160601/eaad5c20/attachment.html>