Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

__builtin_shufflevector runs into an assertion if no integer expressions are passed #92342

Open
tbaederr opened this issue May 16, 2024 · 8 comments
Assignees
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" good first issue https://github.com/llvm/llvm-project/contribute

Comments

@tbaederr
Copy link
Contributor

See https://godbolt.org/z/M5x5Kdnc3, which uses a test case adapted from test/Sema/constant_builtins_vector.cpp.

clang++: /root/llvm-project/clang/include/clang/AST/Expr.h:4483: llvm::APSInt clang::ShuffleVectorExpr::getShuffleMaskIdx(const clang::ASTContext&, unsigned int) const: Assertion `(N < NumExprs - 2) && "Shuffle idx out of range!"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: /opt/compiler-explorer/clang-assertions-trunk/bin/clang++ -gdwarf-4 -g -o /app/output.s -mllvm --x86-asm-syntax=intel -S --gcc-toolchain=/opt/compiler-explorer/gcc-snapshot -fcolor-diagnostics -fno-crash-diagnostics -std=c++2b <source>
1.	<source>:22:66: current parser token ';'
 #0 0x00000000039a1548 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x39a1548)
 #1 0x000000000399f22c llvm::sys::CleanupOnSignal(unsigned long) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x399f22c)
 #2 0x00000000038f0288 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #3 0x000072fd01a42520 (/lib/x86_64-linux-gnu/libc.so.6+0x42520)
 #4 0x000072fd01a969fc pthread_kill (/lib/x86_64-linux-gnu/libc.so.6+0x969fc)
 #5 0x000072fd01a42476 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x42476)
 #6 0x000072fd01a287f3 abort (/lib/x86_64-linux-gnu/libc.so.6+0x287f3)
 #7 0x000072fd01a2871b (/lib/x86_64-linux-gnu/libc.so.6+0x2871b)
 #8 0x000072fd01a39e96 (/lib/x86_64-linux-gnu/libc.so.6+0x39e96)

We should just check for this case in Sema and reject it.

@tbaederr tbaederr added good first issue https://github.com/llvm/llvm-project/contribute clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 16, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented May 16, 2024

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

  1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
  2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
  3. Fix the issue locally.
  4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
  5. Create a Git commit.
  6. Run git clang-format HEAD~1 to format your changes.
  7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

@llvmbot
Copy link
Collaborator

llvmbot commented May 16, 2024

@llvm/issue-subscribers-good-first-issue

Author: Timm Baeder (tbaederr)

See https://godbolt.org/z/M5x5Kdnc3, which uses a test case adapted from `test/Sema/constant_builtins_vector.cpp`.
clang++: /root/llvm-project/clang/include/clang/AST/Expr.h:4483: llvm::APSInt clang::ShuffleVectorExpr::getShuffleMaskIdx(const clang::ASTContext&amp;, unsigned int) const: Assertion `(N &lt; NumExprs - 2) &amp;&amp; "Shuffle idx out of range!"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: /opt/compiler-explorer/clang-assertions-trunk/bin/clang++ -gdwarf-4 -g -o /app/output.s -mllvm --x86-asm-syntax=intel -S --gcc-toolchain=/opt/compiler-explorer/gcc-snapshot -fcolor-diagnostics -fno-crash-diagnostics -std=c++2b &lt;source&gt;
1.	&lt;source&gt;:22:66: current parser token ';'
 #<!-- -->0 0x00000000039a1548 llvm::sys::PrintStackTrace(llvm::raw_ostream&amp;, int) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x39a1548)
 #<!-- -->1 0x000000000399f22c llvm::sys::CleanupOnSignal(unsigned long) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x399f22c)
 #<!-- -->2 0x00000000038f0288 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #<!-- -->3 0x000072fd01a42520 (/lib/x86_64-linux-gnu/libc.so.6+0x42520)
 #<!-- -->4 0x000072fd01a969fc pthread_kill (/lib/x86_64-linux-gnu/libc.so.6+0x969fc)
 #<!-- -->5 0x000072fd01a42476 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x42476)
 #<!-- -->6 0x000072fd01a287f3 abort (/lib/x86_64-linux-gnu/libc.so.6+0x287f3)
 #<!-- -->7 0x000072fd01a2871b (/lib/x86_64-linux-gnu/libc.so.6+0x2871b)
 #<!-- -->8 0x000072fd01a39e96 (/lib/x86_64-linux-gnu/libc.so.6+0x39e96)

We should just check for this case in Sema and reject it.

@llvmbot
Copy link
Collaborator

llvmbot commented May 16, 2024

@llvm/issue-subscribers-clang-frontend

Author: Timm Baeder (tbaederr)

See https://godbolt.org/z/M5x5Kdnc3, which uses a test case adapted from `test/Sema/constant_builtins_vector.cpp`.
clang++: /root/llvm-project/clang/include/clang/AST/Expr.h:4483: llvm::APSInt clang::ShuffleVectorExpr::getShuffleMaskIdx(const clang::ASTContext&amp;, unsigned int) const: Assertion `(N &lt; NumExprs - 2) &amp;&amp; "Shuffle idx out of range!"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: /opt/compiler-explorer/clang-assertions-trunk/bin/clang++ -gdwarf-4 -g -o /app/output.s -mllvm --x86-asm-syntax=intel -S --gcc-toolchain=/opt/compiler-explorer/gcc-snapshot -fcolor-diagnostics -fno-crash-diagnostics -std=c++2b &lt;source&gt;
1.	&lt;source&gt;:22:66: current parser token ';'
 #<!-- -->0 0x00000000039a1548 llvm::sys::PrintStackTrace(llvm::raw_ostream&amp;, int) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x39a1548)
 #<!-- -->1 0x000000000399f22c llvm::sys::CleanupOnSignal(unsigned long) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x399f22c)
 #<!-- -->2 0x00000000038f0288 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #<!-- -->3 0x000072fd01a42520 (/lib/x86_64-linux-gnu/libc.so.6+0x42520)
 #<!-- -->4 0x000072fd01a969fc pthread_kill (/lib/x86_64-linux-gnu/libc.so.6+0x969fc)
 #<!-- -->5 0x000072fd01a42476 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x42476)
 #<!-- -->6 0x000072fd01a287f3 abort (/lib/x86_64-linux-gnu/libc.so.6+0x287f3)
 #<!-- -->7 0x000072fd01a2871b (/lib/x86_64-linux-gnu/libc.so.6+0x2871b)
 #<!-- -->8 0x000072fd01a39e96 (/lib/x86_64-linux-gnu/libc.so.6+0x39e96)

We should just check for this case in Sema and reject it.

@vmishelcs
Copy link
Contributor

Hello, I'd like to give this issue a try :)

@vmishelcs
Copy link
Contributor

vmishelcs commented May 21, 2024

@tbaederr Hello! I believe I've located the source of the bug.

In Sema::BuiltinShuffleVector(CallExpr *TheCall) we do the following check.

QualType LHSType = TheCall->getArg(0)->getType();
QualType RHSType = TheCall->getArg(1)->getType();

// ...

if (TheCall->getNumArgs() == 2) {
  if (!RHSType->hasIntegerRepresentation() || RHSType->castAs<VectorType>()->getNumElements() != numElements)
    return ExprError(Diag(TheCall->getBeginLoc(),
                          diag::err_vec_builtin_incompatible_vector)
                     << TheCall->getDirectCallee()
                     << /*isMorethantwoArgs*/ false
                     << SourceRange(TheCall->getArg(1)->getBeginLoc(), TheCall->getArg(1)->getEndLoc()));
  // ...

This is done to "support unary __builtin_shufflevector" according to the comment written in that same method:

// Determine which of the following types of shufflevector we're checking:
// 1) unary, vector mask: (lhs, mask)
// 2) binary, scalar mask: (lhs, rhs, index, ..., index)

However, I'm a little confused as to why. According to the clang 19.0.0 documentation, __builtin_shufflevector has the signature

__builtin_shufflevector(vec1, vec2, index1, index2, ...)

thus there should be no reason it's called with only 2 vectors provided. Could it be that whoever wrote this comment was confusing clang's __builtin_shufflevector with gcc's __builtin_shuffle described here?

@tbaederr
Copy link
Contributor Author

It seems like this was introduced in a011002, so seems intentional.

@vmishelcs
Copy link
Contributor

@tbaederr So do we want to allow the programmer to use __builtin_shufflevector by writing

constexpr vector4char vector4charConst = {0, 1, 2, 3};
constexpr vector4int vector4intMask = {3, 2, 1, 0};

constexpr vector4char vectorShuffle = __builtin_shufflevector(vector4charConst, vector4intMask);

despite the documentation saying this is invalid? Because GCC doesn't allow this either. GCC's implementation expects that you provide two vector arguments followed by indices.

@tbaederr
Copy link
Contributor Author

Even the documentation for llvm::ShuffleVectorInst mentions it only takes two input vectors, so I think it's safe to reject the "unary" version with only one vector.

If no tests break, there's an opportunity for cleanup up the Sema::BuiltinShuffleVector() function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" good first issue https://github.com/llvm/llvm-project/contribute
Projects
None yet
Development

No branches or pull requests

3 participants