Bug 236216 - devel/valgrind: clang 8 crashes during build
Summary: devel/valgrind: clang 8 crashes during build
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-toolchain (Nobody)
URL:
Keywords:
Depends on:
Blocks: 236062
  Show dependency treegraph
 
Reported: 2019-03-04 13:25 UTC by Jan Beich
Modified: 2019-03-20 23:32 UTC (History)
3 users (show)

See Also:
bugzilla: maintainer-feedback? (bdrewery)


Attachments
genoffsets.c (compressed, preprocessed) (30.21 KB, application/x-xz)
2019-03-04 13:27 UTC, Jan Beich
no flags Details
command line args (for clang 8) (2.93 KB, text/plain)
2019-03-04 13:27 UTC, Jan Beich
no flags Details
Reuse existing offsetof macro for clang 8's sake (1.12 KB, patch)
2019-03-04 18:01 UTC, Dimitry Andric
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer freebsd_triage 2019-03-04 13:25:55 UTC
$ poudriere jail -cj clang8 -v projects/clang800-import -m svn+https
$ poudriere testport -j clang8 devel/valgrind
[...]
cc -Wno-long-long -O2 -pipe  -fstack-protector -fno-strict-aliasing  -Wno-tautological-compare -Wno-cast-align -Wno-self-assign -fno-stack-protector -no-integrated-as \
      -Wbad-function-cast -Wcast-qual -Wcast-align -fstrict-aliasing \
      -m64 -O2 -g -Wall -Wmissing-prototypes -Wshadow -Wpointer-arith -Wstrict-prototypes -Wmissing-declarations -Wno-format-zero-length -Wno-tautological-compare -fno-strict-aliasing -fno-builtin -fomit-frame-pointer \
			-O -S -o auxprogs/genoffsets.s \
				 ./auxprogs/genoffsets.c
Assertion failed: (isInt() && "Invalid accessor"), function getInt, file /poudriere/jails/headamd64PR236062/usr/src/contrib/llvm/tools/clang/include/clang/AST/APValue.h, line 253.
cc: error: unable to execute command: Abort trap (core dumped)
cc: error: clang frontend command failed due to signal (use -v to see invocation)
FreeBSD clang version 8.0.0 (branches/release_80 354799) (based on LLVM 8.0.0)
Target: x86_64-unknown-freebsd13.0
Thread model: posix
InstalledDir: /usr/bin

http://package18.nyi.freebsd.org/data/headamd64PR236062-default/2019-03-01_06h42m53s/logs/errors/valgrind-3.10.1.20160113_7,1.log
http://package18.nyi.freebsd.org/data/headamd64PR236062-default/2019-03-01_06h42m53s/logs/errors/valgrind-devel-3.10.1.20160113_6,1.log
Comment 1 Jan Beich freebsd_committer freebsd_triage 2019-03-04 13:27:16 UTC
Created attachment 202548 [details]
genoffsets.c (compressed, preprocessed)
Comment 2 Jan Beich freebsd_committer freebsd_triage 2019-03-04 13:27:45 UTC
Created attachment 202549 [details]
command line args (for clang 8)
Comment 3 Jan Beich freebsd_committer freebsd_triage 2019-03-04 13:31:17 UTC
Need someone to bisect Clang and/or analyze before patching this port.
Comment 4 Dimitry Andric freebsd_committer freebsd_triage 2019-03-04 18:01:23 UTC
Created attachment 202554 [details]
Reuse existing offsetof macro for clang 8's sake

This is regressed by upstream https://reviews.llvm.org/rL349561 ("Emit ASM input in a constant context"), and I already filed an upstream bug here:

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

The cause is the custom my_offsetof() macro in genoffsets, which returns a pointer, not an offset.  The attached patch makes it reuse the already existing offsetof() macro from VEX/pub/libvex_basictypes.h, which does the right thing.

I think this is an acceptable workaround until upstream fixes the issue with <https://reviews.llvm.org/rL349561>, or even as a permanent cleanup.
Comment 5 Jan Beich freebsd_committer freebsd_triage 2019-03-04 23:31:14 UTC
Reminder: Clang 8 was merged to 13.0-CURRENT in base r344779. Maintainers, expect pkg-fallout@ mail soon.
Comment 6 commit-hook freebsd_committer freebsd_triage 2019-03-07 19:33:46 UTC
A commit references this bug:

Author: dim
Date: Thu Mar  7 19:33:40 UTC 2019
New revision: 344896
URL: https://svnweb.freebsd.org/changeset/base/344896

Log:
  ?Pull in r354937 from upstream clang trunk (by J?rg Sonnenberger):

    Fix inline assembler constraint validation

    The current constraint logic is both too lax and too strict. It fails
    for input outside the [INT_MIN..INT_MAX] range, but it also
    implicitly accepts 0 as value when it should not. Adjust logic to
    handle both correctly.

    Differential Revision: https://reviews.llvm.org/D58649

  Pull in r355491 from upstream clang trunk (by Hans Wennborg):

    Inline asm constraints: allow ICE-like pointers for the "n"
    constraint (PR40890)

    Apparently GCC allows this, and there's code relying on it (see bug).

    The idea is to allow expression that would have been allowed if they
    were cast to int. So I based the code on how such a cast would be
    done (the CK_PointerToIntegral case in
    IntExprEvaluator::VisitCastExpr()).

    Differential Revision: https://reviews.llvm.org/D58821

  These should fix assertions and errors when using the inline assembly
  "n" constraint in certain ways.

  In case of devel/valgrind, a pointer was used as the input for the
  constraint, which lead to "Assertion failed: (isInt() && "Invalid
  accessor"), function getInt".

  In case of math/secp256k1, a very large integer value was used as input
  for the constraint, which lead to "error: value '4624529908474429119'
  out of range for constraint 'n'".

  PR:             236216, 236194
  MFC after:      1 month
  X-MFC-With:     r344779

Changes:
  head/contrib/llvm/tools/clang/include/clang/AST/APValue.h
  head/contrib/llvm/tools/clang/include/clang/Basic/TargetInfo.h
  head/contrib/llvm/tools/clang/lib/AST/APValue.cpp
  head/contrib/llvm/tools/clang/lib/AST/ExprConstant.cpp
  head/contrib/llvm/tools/clang/lib/CodeGen/CGStmt.cpp
  head/contrib/llvm/tools/clang/lib/Sema/SemaStmtAsm.cpp
Comment 7 Dimitry Andric freebsd_committer freebsd_triage 2019-03-07 19:39:12 UTC
So after r344896, no more workaround should be needed.  (That said, the duplicated offsetof macro in getoffsetof is still a wart, and should be fixed upstream. :)