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

Undefined reference when using atomics (LLVM-58) #3

Closed
lexxvir opened this issue Apr 9, 2019 · 19 comments
Closed

Undefined reference when using atomics (LLVM-58) #3

lexxvir opened this issue Apr 9, 2019 · 19 comments

Comments

@lexxvir
Copy link

lexxvir commented Apr 9, 2019

Hi,

I encountered into issue when try to use Rust compiler's AtomicBool.

Linker error: undefined reference to __sync_lock_test_and_set_1.
Quite same error for AtomicUsize: undefined reference to __sync_lock_test_and_set_4.

It seems that LLVM somehow should replace these symbols by real atomic instructions for xtensa target.

@pacmancoder
Copy link

Does anyone found workarounds for Atomics support in Rust for the current LLVM state?

@lexxvir
Copy link
Author

lexxvir commented Aug 31, 2019

Personally I replaced all Atomics by FreeRTOS Mutexes, though I have only few Atomics in whole codebase.

@pacmancoder
Copy link

I can imagine that it is possible to write a bit of shim code in C with it's bultins and then link this library with rust application, but I am not tried this yet.

Yes, we can use free rtos sync primitives, but anscense of rust atomics is blocking use of the Arc from alloc crate, which is a bit sad. :(

@pacmancoder
Copy link

pacmancoder commented Aug 31, 2019

@lexxvir Looks like my idea is working. I made shim c-code to call gcc builtins and exported these functions in the rust code. With this shim layer, code using atomics can be compiled.

Here is my gist:
https://gist.github.com/pacmancoder/70788fc112b582f71f885e2c3104c0ab

I made .c and .rs files generation script in python (btw, sorry for the very dirty code).

I am not performed a lot of testing yet, but I will try integrate this worarround in my current work in idf-hal/idf-sys projects (yep, shameless plug) and will tell if it works correct.

UPD: Performed generation script refactoring. Now it looks more adequate 🛩️

@lexxvir
Copy link
Author

lexxvir commented Sep 2, 2019

@pacmancoder I have not experience in such low level code, so I'm probably not right, but is such shims must be marked as "inline always" both in C and in Rust? Or inlining will be done by compiler auto-magically?

BTW, just FYI, there are wrapper crates for ESP IDF made by @sapir: esp-idf-sys, esp-idf-hal.

@pacmancoder
Copy link

@lexxvir specifying code inlining for this case is quite problematic, as C-shims should be represented as valid functions to be able to link them from the rust side. I think this inlining can be handled automatically by the compiler on the link stage when -flto (link time optimizations) is specified. (related thread).

Anyway, it is just a quick and dirty worarround, and I hope the issue with atomics will be fixed in LLVM-Xtensa

About the projects by @sapir: I know about them, he did a great work 👍 ! But I decided to write my own version of the crates with more configurability (to not bound user to the specific sdkconfig and even platform (I plan to support both esp8266 and esp32) and with less unsafe code (owned periperals singletone and direct ownership of the separate peripherals). Because of lack of the time, its development is going very slow (For now, I implemented only wifi ap/sta configuration) but I hope I will make it usable in a few months

@igrr igrr transferred this issue from espressif/llvm-xtensa Jan 15, 2020
@espressif-bot espressif-bot changed the title Undefined reference when using atomics Undefined reference when using atomics (LLVM-58) Jan 29, 2020
@vgarleanu
Copy link

pinging @lexxvir and @pacmancoder . Whats the status on this? I am still getting this linking error when using rustc

@lexxvir
Copy link
Author

lexxvir commented Apr 29, 2020

Hi @vgarleanu !

I just tested it again, on recent compiler (see #20) and it's OK. Code links successfully and runs without issue. I my test I just incrementing AtomicUsize in the loop by the two threads and output the values.

@vgarleanu
Copy link

vgarleanu commented Apr 29, 2020

@lexxvir Thanks a lot for taking your time to test it out on your system. I just did some more testing myself and it seems that only AtomicBool causes a linker error throwing undefined reference to __sync_lock_test_and_set_1'. AtomicUsize works fine.
UPDATE: It actually turns out that the linker error is caused when the swap method of any Atomic is used.
Do you mind testing it on your side? FYI my snippet is just this:

#[no_mangle]
pub fn app_main() {
    static FIRST_PANIC: AtomicBool = AtomicBool::new(true);
    FIRST_PANIC.swap(false, SeqCst);
}

P.S. I am also using the latest commit of https://github.com/MabezDev/rust-xtensa. I just placed that snippet inside the main.rs of https://github.com/lexxvir/esp32-hello

@lexxvir
Copy link
Author

lexxvir commented Apr 29, 2020

@vgarleanu Yeah, you're right, swap leads to the linker error for me too:
__sync_lock_test_and_set_1 for AtomicBool and __sync_lock_test_and_set_4 for AtomicUsize.

Just in case, my test:
atomic.zip

@vgarleanu
Copy link

@lexxvir Alright, how do we go about tracking the cause down? Is there any maintainer from espressif that we can ping in?

@lexxvir
Copy link
Author

lexxvir commented Apr 29, 2020

@vgarleanu I think @andreisfr or @igrr could help here...

@vgarleanu
Copy link

Should i perhaps open a separate issue? Im unsure if this bug relates with the one atop.

@reitermarkus
Copy link

@vgarleanu, I think we are all talking about the same bug. I can confirm this bug still exists in the newest version. I am still using the C shim @pacmancoder mentioned as a workaround for now.

@andreisfr
Copy link
Collaborator

@lexxvir , @vgarleanu , thank you for information, we investigate the problem.

@vgarleanu
Copy link

hey @andreisfr whats the status on this?

@lexxvir
Copy link
Author

lexxvir commented Dec 13, 2020

Hi there,

I've just built new Rust compiler using xtensa_release_11.0.0 branch and it seems compiler now has atomics!
To check that I've used test from #3 (comment).

@vgarleanu
Copy link

Gotcha, ill check everything on my side sometime this week!

@gerekon gerekon closed this as completed May 18, 2021
@gerekon
Copy link
Collaborator

gerekon commented May 18, 2021

igrr pushed a commit that referenced this issue Dec 3, 2021
This patch re-introduces the fix in the commit llvm@66b0cebf7f736 by @yrnkrn

> In DwarfEHPrepare, after all passes are run, RewindFunction may be a dangling
>
> pointer to a dead function. To make sure it's valid, doFinalization nullptrs
> RewindFunction just like the constructor and so it will be found on next run.
>
> llvm-svn: 217737

It seems that the fix was not migrated to `DwarfEHPrepareLegacyPass`.

This patch also updates `llvm/test/CodeGen/X86/dwarf-eh-prepare.ll` to include `-run-twice` to exercise the cleanup. Without this patch `llvm-lit -v llvm/test/CodeGen/X86/dwarf-eh-prepare.ll` fails with

```
-- Testing: 1 tests, 1 workers --
FAIL: LLVM :: CodeGen/X86/dwarf-eh-prepare.ll (1 of 1)
******************** TEST 'LLVM :: CodeGen/X86/dwarf-eh-prepare.ll' FAILED ********************
Script:
--
: 'RUN: at line 1';   /home/arakaki/build/llvm-project/main/bin/opt -mtriple=x86_64-linux-gnu -dwarfehprepare -simplifycfg-require-and-preserve-domtree=1 -run-twice < /home/arakaki/repos/watch/llvm-project/llvm/test/CodeGen/X86/dwarf-eh-prepare.ll -S | /home/arakaki/build/llvm-project/main/bin/FileCheck /home/arakaki/repos/watch/llvm-project/llvm/test/CodeGen/X86/dwarf-eh-prepare.ll
--
Exit Code: 2

Command Output (stderr):
--
Referencing function in another module!
  call void @_Unwind_Resume(i8* %ehptr) #1
; ModuleID = '<stdin>'
void (i8*)* @_Unwind_Resume
; ModuleID = '<stdin>'
in function simple_cleanup_catch
LLVM ERROR: Broken function found, compilation aborted!
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.      Program arguments: /home/arakaki/build/llvm-project/main/bin/opt -mtriple=x86_64-linux-gnu -dwarfehprepare -simplifycfg-require-and-preserve-domtree=1 -run-twice -S
1.      Running pass 'Function Pass Manager' on module '<stdin>'.
2.      Running pass 'Module Verifier' on function '@simple_cleanup_catch'
 #0 0x000056121b570a2c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/arakaki/repos/watch/llvm-project/llvm/lib/Support/Unix/Signals.inc:569:0
 #1 0x000056121b56eb64 llvm::sys::RunSignalHandlers() /home/arakaki/repos/watch/llvm-project/llvm/lib/Support/Signals.cpp:97:0
 #2 0x000056121b56f28e SignalHandler(int) /home/arakaki/repos/watch/llvm-project/llvm/lib/Support/Unix/Signals.inc:397:0
 #3 0x00007fc7e9b22980 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12980)
 #4 0x00007fc7e87d3fb7 raise /build/glibc-S7xCS9/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
 #5 0x00007fc7e87d5921 abort /build/glibc-S7xCS9/glibc-2.27/stdlib/abort.c:81:0
 #6 0x000056121b4e1386 llvm::raw_svector_ostream::raw_svector_ostream(llvm::SmallVectorImpl<char>&) /home/arakaki/repos/watch/llvm-project/llvm/include/llvm/Support/raw_ostream.h:674:0
 #7 0x000056121b4e1386 llvm::report_fatal_error(llvm::Twine const&, bool) /home/arakaki/repos/watch/llvm-project/llvm/lib/Support/ErrorHandling.cpp:114:0
 #8 0x000056121b4e1528 (/home/arakaki/build/llvm-project/main/bin/opt+0x29e3528)
 #9 0x000056121adfd03f llvm::raw_ostream::operator<<(llvm::StringRef) /home/arakaki/repos/watch/llvm-project/llvm/include/llvm/Support/raw_ostream.h:218:0
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /home/arakaki/build/llvm-project/main/bin/FileCheck /home/arakaki/repos/watch/llvm-project/llvm/test/CodeGen/X86/dwarf-eh-prepare.ll

--

********************
********************
Failed Tests (1):
  LLVM :: CodeGen/X86/dwarf-eh-prepare.ll

Testing Time: 0.22s
  Failed: 1
```

Reviewed By: loladiro

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

(cherry picked from commit e8806d7)
igrr pushed a commit that referenced this issue May 2, 2022
…ified offset and its parents or children with spcified depth."

This reverts commit a3b7cb0.

symbol-offset.test fails under MSAN:

[  1] ; RUN: llvm-pdbutil yaml2pdb %p/Inputs/symbol-offset.yaml --pdb=%t.pdb [FAIL]
llvm-pdbutil yaml2pdb <REDACTED>/llvm/test/tools/llvm-pdbutil/Inputs/symbol-offset.yaml --pdb=<REDACTED>/tmp/symbol-offset.test/symbol-offset.test.tmp.pdb
==9283==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x55f975e5eb91 in __libcpp_tls_set <REDACTED>/include/c++/v1/__threading_support:428:12
    #1 0x55f975e5eb91 in set_pointer <REDACTED>/include/c++/v1/thread:196:5
    #2 0x55f975e5eb91 in void* std::__msan::__thread_proxy<std::__msan::tuple<std::__msan::unique_ptr<std::__msan::__thread_struct, std::__msan::default_delete<std::__msan::__thread_struct> >, llvm::parallel::detail::(anonymous namespace)::ThreadPoolExecutor::ThreadPoolExecutor(llvm::ThreadPoolStrategy)::'lambda'()::operator()() const::'lambda'()> >(void*) <REDACTED>/include/c++/v1/thread:285:27
    #3 0x7f74a1e55b54 in start_thread (<REDACTED>/libpthread.so.0+0xbb54) (BuildId: 64752de50ebd1a108f4b3f8d0d7e1a13)
    #4 0x7f74a1dc9f7e in clone (<REDACTED>/libc.so.6+0x13cf7e) (BuildId: 7cfed7708e5ab7fcb286b373de21ee76)
igrr pushed a commit that referenced this issue May 2, 2022
The asm parser had a notional distinction between parsing an
operand (like "%foo" or "%4#3") and parsing a region argument
(which isn't supposed to allow a result number like #3).

Unfortunately the implementation has two problems:

1) It didn't actually check for the result number and reject
   it.  parseRegionArgument and parseOperand were identical.
2) It had a lot of machinery built up around it that paralleled
   operand parsing.  This also was functionally identical, but
   also had some subtle differences (e.g. the parseOptional
   stuff had a different result type).

I thought about just removing all of this, but decided that the
missing error checking was important, so I reimplemented it with
a `allowResultNumber` flag on parseOperand.  This keeps the
codepaths unified and adds the missing error checks.

Differential Revision: https://reviews.llvm.org/D124470
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants