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

Can't legalize widths between legal widths. #12

Open
jacobly0 opened this issue Sep 7, 2020 · 0 comments
Open

Can't legalize widths between legal widths. #12

jacobly0 opened this issue Sep 7, 2020 · 0 comments
Assignees
Labels
bug Something isn't working crash Something crashed enhancement New feature or request

Comments

@jacobly0
Copy link
Owner

jacobly0 commented Sep 7, 2020

Repro -O1

long a;char b;void c(void){a=0;for(;a!=4;++a){a||(b=0);}}

Since that doesn't repro anymore, here's a direct repro:

target triple = "ez80"
define i9 @test() {
  ret i9 0
}

C repro -O0:

struct { int : 18, a : 15; } b; c() { return b.a; }
@jacobly0 jacobly0 added bug Something isn't working crash Something crashed labels Sep 7, 2020
@jacobly0 jacobly0 self-assigned this Sep 7, 2020
jacobly0 pushed a commit that referenced this issue Dec 11, 2020
CXXDeductionGuideDecl with a local typedef has its own copy of the
TypedefDecl with the CXXDeductionGuideDecl as the DeclContext of that
TypedefDecl.
```
      template <typename T> struct A {
        typedef T U;
        A(U, T);
      };
      A a{(int)0, (int)0};
```
Related discussion on cfe-dev:
http://lists.llvm.org/pipermail/cfe-dev/2020-November/067252.html

Without this fix, when we import the CXXDeductionGuideDecl (via
VisitFunctionDecl) then before creating the Decl we must import the
FunctionType. However, the first parameter's type is the afore mentioned
local typedef. So, we then start importing the TypedefDecl whose
DeclContext is the CXXDeductionGuideDecl itself. The infinite loop is
formed.
```
 #0 clang::ASTNodeImporter::VisitCXXDeductionGuideDecl(clang::CXXDeductionGuideDecl*) clang/lib/AST/ASTImporter.cpp:3543:0
 #1 clang::declvisitor::Base<std::add_pointer, clang::ASTNodeImporter, llvm::Expected<clang::Decl*> >::Visit(clang::Decl*) /home/egbomrt/WORK/llvm5/build/debug/tools/clang/include/clang/AST/DeclNodes.inc:405:0
 #2 clang::ASTImporter::ImportImpl(clang::Decl*) clang/lib/AST/ASTImporter.cpp:8038:0
 #3 clang::ASTImporter::Import(clang::Decl*) clang/lib/AST/ASTImporter.cpp:8200:0
 #4 clang::ASTImporter::ImportContext(clang::DeclContext*) clang/lib/AST/ASTImporter.cpp:8297:0
 #5 clang::ASTNodeImporter::ImportDeclContext(clang::Decl*, clang::DeclContext*&, clang::DeclContext*&) clang/lib/AST/ASTImporter.cpp:1852:0
 #6 clang::ASTNodeImporter::ImportDeclParts(clang::NamedDecl*, clang::DeclContext*&, clang::DeclContext*&, clang::DeclarationName&, clang::NamedDecl*&, clang::SourceLocation&) clang/lib/AST/ASTImporter.cpp:1628:0
 #7 clang::ASTNodeImporter::VisitTypedefNameDecl(clang::TypedefNameDecl*, bool) clang/lib/AST/ASTImporter.cpp:2419:0
 #8 clang::ASTNodeImporter::VisitTypedefDecl(clang::TypedefDecl*) clang/lib/AST/ASTImporter.cpp:2500:0
 #9 clang::declvisitor::Base<std::add_pointer, clang::ASTNodeImporter, llvm::Expected<clang::Decl*> >::Visit(clang::Decl*) /home/egbomrt/WORK/llvm5/build/debug/tools/clang/include/clang/AST/DeclNodes.inc:315:0
 #10 clang::ASTImporter::ImportImpl(clang::Decl*) clang/lib/AST/ASTImporter.cpp:8038:0
 #11 clang::ASTImporter::Import(clang::Decl*) clang/lib/AST/ASTImporter.cpp:8200:0
 #12 llvm::Expected<clang::TypedefNameDecl*> clang::ASTNodeImporter::import<clang::TypedefNameDecl>(clang::TypedefNameDecl*) clang/lib/AST/ASTImporter.cpp:165:0
 #13 clang::ASTNodeImporter::VisitTypedefType(clang::TypedefType const*) clang/lib/AST/ASTImporter.cpp:1304:0
 #14 clang::TypeVisitor<clang::ASTNodeImporter, llvm::Expected<clang::QualType> >::Visit(clang::Type const*) /home/egbomrt/WORK/llvm5/build/debug/tools/clang/include/clang/AST/TypeNodes.inc:74:0
 #15 clang::ASTImporter::Import(clang::QualType) clang/lib/AST/ASTImporter.cpp:8071:0
 #16 llvm::Expected<clang::QualType> clang::ASTNodeImporter::import<clang::QualType>(clang::QualType const&) clang/lib/AST/ASTImporter.cpp:179:0
 #17 clang::ASTNodeImporter::VisitFunctionProtoType(clang::FunctionProtoType const*) clang/lib/AST/ASTImporter.cpp:1244:0
 #18 clang::TypeVisitor<clang::ASTNodeImporter, llvm::Expected<clang::QualType> >::Visit(clang::Type const*) /home/egbomrt/WORK/llvm5/build/debug/tools/clang/include/clang/AST/TypeNodes.inc:47:0
 #19 clang::ASTImporter::Import(clang::QualType) clang/lib/AST/ASTImporter.cpp:8071:0
 #20 llvm::Expected<clang::QualType> clang::ASTNodeImporter::import<clang::QualType>(clang::QualType const&) clang/lib/AST/ASTImporter.cpp:179:0
 #21 clang::QualType clang::ASTNodeImporter::importChecked<clang::QualType>(llvm::Error&, clang::QualType const&) clang/lib/AST/ASTImporter.cpp:198:0
 #22 clang::ASTNodeImporter::VisitFunctionDecl(clang::FunctionDecl*) clang/lib/AST/ASTImporter.cpp:3313:0
 #23 clang::ASTNodeImporter::VisitCXXDeductionGuideDecl(clang::CXXDeductionGuideDecl*) clang/lib/AST/ASTImporter.cpp:3543:0
```

The fix is to first create the TypedefDecl and only then start to import
the DeclContext.
Basically, we could do this during the import of all other Decls (not
just for typedefs). But it seems, there is only one another AST
construct that has a similar cycle: a struct defined as a function
parameter:
```
int struct_in_proto(struct data_t{int a;int b;} *d);

```
In that case, however, we had decided to return simply with an error
back then because that seemed to be a very rare construct.

Differential Revision: https://reviews.llvm.org/D92209
@jacobly0 jacobly0 added the enhancement New feature or request label Jan 31, 2022
jacobly0 added a commit that referenced this issue Jan 31, 2022
@jacobly0 jacobly0 linked a pull request Jan 31, 2022 that will close this issue
jacobly0 pushed a commit that referenced this issue Feb 8, 2022
We experienced some deadlocks when we used multiple threads for logging
using `scan-builds` intercept-build tool when we used multiple threads by
e.g. logging `make -j16`

```
(gdb) bt
#0  0x00007f2bb3aff110 in __lll_lock_wait () from /lib/x86_64-linux-gnu/libpthread.so.0
#1  0x00007f2bb3af70a3 in pthread_mutex_lock () from /lib/x86_64-linux-gnu/libpthread.so.0
#2  0x00007f2bb3d152e4 in ?? ()
#3  0x00007ffcc5f0cc80 in ?? ()
#4  0x00007f2bb3d2bf5b in ?? () from /lib64/ld-linux-x86-64.so.2
#5  0x00007f2bb3b5da27 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#6  0x00007f2bb3b5dbe0 in exit () from /lib/x86_64-linux-gnu/libc.so.6
#7  0x00007f2bb3d144ee in ?? ()
#8  0x746e692f706d742f in ?? ()
#9  0x692d747065637265 in ?? ()
#10 0x2f653631326b3034 in ?? ()
#11 0x646d632e35353532 in ?? ()
#12 0x0000000000000000 in ?? ()
```

I think the gcc's exit call caused the injected `libear.so` to be unloaded
by the `ld`, which in turn called the `void on_unload() __attribute__((destructor))`.
That tried to acquire an already locked mutex which was left locked in the
`bear_report_call()` call, that probably encountered some error and
returned early when it forgot to unlock the mutex.

All of these are speculation since from the backtrace I could not verify
if frames 2 and 3 are in fact corresponding to the `libear.so` module.
But I think it's a fairly safe bet.

So, hereby I'm releasing the held mutex on *all paths*, even if some failure
happens.

PS: I would use lock_guards, but it's C.

Reviewed-by: NoQ

Differential Revision: https://reviews.llvm.org/D118439
nebulatgs pushed a commit to nebulatgs/llvm-project that referenced this issue Mar 23, 2022
We experienced some deadlocks when we used multiple threads for logging
using `scan-builds` intercept-build tool when we used multiple threads by
e.g. logging `make -j16`

```
(gdb) bt
#0  0x00007f2bb3aff110 in __lll_lock_wait () from /lib/x86_64-linux-gnu/libpthread.so.0
jacobly0#1  0x00007f2bb3af70a3 in pthread_mutex_lock () from /lib/x86_64-linux-gnu/libpthread.so.0
jacobly0#2  0x00007f2bb3d152e4 in ?? ()
jacobly0#3  0x00007ffcc5f0cc80 in ?? ()
jacobly0#4  0x00007f2bb3d2bf5b in ?? () from /lib64/ld-linux-x86-64.so.2
jacobly0#5  0x00007f2bb3b5da27 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
jacobly0#6  0x00007f2bb3b5dbe0 in exit () from /lib/x86_64-linux-gnu/libc.so.6
jacobly0#7  0x00007f2bb3d144ee in ?? ()
jacobly0#8  0x746e692f706d742f in ?? ()
jacobly0#9  0x692d747065637265 in ?? ()
jacobly0#10 0x2f653631326b3034 in ?? ()
jacobly0#11 0x646d632e35353532 in ?? ()
jacobly0#12 0x0000000000000000 in ?? ()
```

I think the gcc's exit call caused the injected `libear.so` to be unloaded
by the `ld`, which in turn called the `void on_unload() __attribute__((destructor))`.
That tried to acquire an already locked mutex which was left locked in the
`bear_report_call()` call, that probably encountered some error and
returned early when it forgot to unlock the mutex.

All of these are speculation since from the backtrace I could not verify
if frames 2 and 3 are in fact corresponding to the `libear.so` module.
But I think it's a fairly safe bet.

So, hereby I'm releasing the held mutex on *all paths*, even if some failure
happens.

PS: I would use lock_guards, but it's C.

Reviewed-by: NoQ

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

(cherry picked from commit d919d02)
jacobly0 pushed a commit that referenced this issue Jun 4, 2022
For functions that require restoring SP from FP (e.g. that need to
align the stack, or that have variable sized allocations), the prologue
and epilogue previously used to look like this:

    push {r4-r5, r11, lr}
    add r11, sp, #8
    ...
    sub r4, r11, #8
    mov sp, r4
    pop {r4-r5, r11, pc}

This is problematic, because this unwinding operation (restoring sp
from r11 - offset) can't be expressed with the SEH unwind opcodes
(probably because this unwind procedure doesn't map exactly to
individual instructions; note the detour via r4 in the epilogue too).

To make unwinding work, the GPR push is split into two; the first one
pushing all other registers, and the second one pushing r11+lr, so that
r11 can be set pointing at this spot on the stack:

    push {r4-r5}
    push {r11, lr}
    mov r11, sp
    ...
    mov sp, r11
    pop {r11, lr}
    pop {r4-r5}
    bx lr

For the same setup, MSVC generates code that uses two registers;
r11 still pointing at the {r11,lr} pair, but a separate register
used for restoring the stack at the end:

    push {r4-r5, r7, r11, lr}
    add r11, sp, #12
    mov r7, sp
    ...
    mov sp, r7
    pop {r4-r5, r7, r11, pc}

For cases with clobbered float/vector registers, they are pushed
after the GPRs, before the {r11,lr} pair.

Differential Revision: https://reviews.llvm.org/D125649
jacobly0 pushed a commit that referenced this issue Jun 4, 2022
Previously, the "add sp, #12" ended up inserted after "bx lr".

Differential Revision: https://reviews.llvm.org/D126872
@adriweb adriweb mentioned this issue Apr 29, 2023
jacobly0 added a commit that referenced this issue Nov 6, 2023
jacobly0 pushed a commit that referenced this issue Nov 20, 2024
…onger cause a crash (llvm#116569)

This PR fixes a bug introduced by llvm#110199, which causes any half float
argument to crash the compiler on MIPS64.

Currently compiling this bit of code with `llc -mtriple=mips64`: 
```
define void @half_args(half %a) nounwind {
entry:
        ret void
}
```

Crashes with the following log:
```
LLVM ERROR: unable to allocate function argument #0
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: llc -mtriple=mips64
1.	Running pass 'Function Pass Manager' on module '<stdin>'.
2.	Running pass 'MIPS DAG->DAG Pattern Instruction Selection' on function '@half_args'
 #0 0x000055a3a4013df8 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x32d0df8)
 #1 0x000055a3a401199e llvm::sys::RunSignalHandlers() (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x32ce99e)
 #2 0x000055a3a40144a8 SignalHandler(int) Signals.cpp:0:0
 #3 0x00007f00bde558c0 __restore_rt libc_sigaction.c:0:0
 #4 0x00007f00bdea462c __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
 #5 0x00007f00bde55822 gsignal ./signal/../sysdeps/posix/raise.c:27:6
 #6 0x00007f00bde3e4af abort ./stdlib/abort.c:81:7
 #7 0x000055a3a3f80e3c llvm::report_fatal_error(llvm::Twine const&, bool) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x323de3c)
 #8 0x000055a3a2e20dfa (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x20dddfa)
 #9 0x000055a3a2a34e20 llvm::MipsTargetLowering::LowerFormalArguments(llvm::SDValue, unsigned int, bool, llvm::SmallVectorImpl<llvm::ISD::InputArg> const&, llvm::SDLoc const&, llvm::SelectionDAG&, llvm::SmallVectorImpl<llvm::SDValue>&) const MipsISelLowering.cpp:0:0
#10 0x000055a3a3d896a9 llvm::SelectionDAGISel::LowerArguments(llvm::Function const&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30466a9)
#11 0x000055a3a3e0b3ec llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30c83ec)
#12 0x000055a3a3e09e21 llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30c6e21)
#13 0x000055a3a2aae1ca llvm::MipsDAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&) MipsISelDAGToDAG.cpp:0:0
#14 0x000055a3a3e07706 llvm::SelectionDAGISelLegacy::runOnMachineFunction(llvm::MachineFunction&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x30c4706)
#15 0x000055a3a3051ed6 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x230eed6)
#16 0x000055a3a35a3ec9 llvm::FPPassManager::runOnFunction(llvm::Function&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x2860ec9)
#17 0x000055a3a35ac3b2 llvm::FPPassManager::runOnModule(llvm::Module&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x28693b2)
#18 0x000055a3a35a499c llvm::legacy::PassManagerImpl::run(llvm::Module&) (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x286199c)
#19 0x000055a3a262abbb main (/home/davide/Ps2/rps2-tools/prefix/bin/llc+0x18e7bbb)
#20 0x00007f00bde3fc4c __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:74:3
#21 0x00007f00bde3fd05 call_init ./csu/../csu/libc-start.c:128:20
#22 0x00007f00bde3fd05 __libc_start_main@GLIBC_2.2.5 ./csu/../csu/libc-start.c:347:5
#23 0x000055a3a2624921 _start /builddir/glibc-2.39/csu/../sysdeps/x86_64/start.S:117:0
```

This is caused by the fact that after the change, `f16`s are no longer
lowered as `f32`s in calls.

Two possible fixes are available:
- Update calling conventions to properly support passing `f16` as
integers.
- Update `useFPRegsForHalfType()` to return `true` so that `f16` are
still kept in `f32` registers, as before llvm#110199.

This PR implements the first solution to not introduce any more ABI
changes as llvm#110199 already did.

As of what is the correct ABI for halfs, I don't think there is a
correct answer. GCC doesn't support halfs on MIPS, and I couldn't find
any information on old MIPS ABI manuals either.
jacobly0 pushed a commit that referenced this issue Dec 21, 2024
…ns (llvm#120564)

This patch makes sure that `BinaryContext::printInstruction` prints the
preferred disassembly. Preferred disassembly only gets printed when
there are no annotations on the MCInst. Therefore, this patch
temporarily removes the annotations before printing it.

A few examples of before and after on AArch64 instructions are as
follows:

```
  BEFORE                     AFTER
                             (preferred disassembly)

  ret   x30                  ret
  orr   x30, xzr, x0         mov   x30, x0
  hint  #29                  autiasp
  hint  #12                  autia1716
```

Clearly, the preferred disassembly is easier for developers to read, and
is the disassembly that tools should be printing.

This patch is motivated as part of future work on the
llvm-bolt-binary-analysis tool, making sure that the reports it prints
do use preferred disassembly.

This patch was cherry-picked from
https://github.com/kbeyls/llvm-project/tree/bolt-gadget-scanner-prototype.

In this current patch, this only affects existing RISCV test cases.

This patch also does improve test cases in future patches that will
introduce a binary analysis for llvm-bolt-binary-analysis that checks
for correct application of pac-ret (pointer authentication on return
addresses).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working crash Something crashed enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant