Skip to content

Support swiftself and swifterror for WebAssembly #3

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

Conversation

kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Mar 12, 2020

This patch is also sent to llvm-project upstream
https://reviews.llvm.org/D76049

Overview

Support signature difference for swiftself and swifterror when cc is
swiftcc.

e.g.

declare swiftcc void @foo(i32, i32)
@data = global i8* bitcast (void (i32, i32)* @foo to i8*)
define swiftcc void @bar() {
  %1 = load i8*, i8** @data
  %2 = bitcast i8* %1 to void (i32, i32, i32)*
  call swiftcc void %2(i32 1, i32 2, i32 swiftself 3)
  ret void
}

For swiftcc, emit additional swiftself and swifterror parameters
if there aren't while lowering. These additional parameters are added
for both callee and caller.
They are necessary to match callee and caller signature for indirect call.

@kateinoigakukun kateinoigakukun force-pushed the katei/swiftwasm/swiftcc-lowering branch from ef556c5 to 566b3b0 Compare March 12, 2020 08:28
@kateinoigakukun kateinoigakukun force-pushed the katei/swiftwasm/swiftcc-lowering branch from 566b3b0 to 21e9e4f Compare March 12, 2020 09:34
@kateinoigakukun kateinoigakukun force-pushed the katei/swiftwasm/swiftcc-lowering branch from 21e9e4f to b6e5f82 Compare March 13, 2020 06:24
@kateinoigakukun
Copy link
Member Author

@MaxDesiatov Can I merge this before https://reviews.llvm.org/D76049 is merged into upstream?

Copy link

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general seems legit, but I don't have experience with this area. Just a few minor suggestions for the comments.

@kateinoigakukun kateinoigakukun force-pushed the katei/swiftwasm/swiftcc-lowering branch from b6e5f82 to 6885af6 Compare March 13, 2020 15:18
@kateinoigakukun
Copy link
Member Author

kateinoigakukun commented Mar 13, 2020

@MaxDesiatov Thanks 😄 I'll revert or fix this if there is a problem, so I'll merge this at this time.

Support signature difference for swiftself and swifterror when cc is
swiftcc.

e.g.
```llvm
declare swiftcc void @foo(i32, i32)
@DaTa = global i8* bitcast (void (i32, i32)* @foo to i8*)
define swiftcc void @bar() {
  %1 = load i8*, i8** @DaTa
  %2 = bitcast i8* %1 to void (i32, i32, i32)*
  call swiftcc void %2(i32 1, i32 2, i32 swiftself 3)
  ret void
}
```

For swiftcc, emit additional swiftself and swifterror parameters
if there aren't while lowering. These additional parameters are added
for both callee and caller.
They are necessary to match callee and caller signature for indirect call.
@kateinoigakukun kateinoigakukun force-pushed the katei/swiftwasm/swiftcc-lowering branch from 6885af6 to b15b427 Compare March 13, 2020 15:36
Copy link

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@kateinoigakukun kateinoigakukun merged commit 933a544 into swiftwasm:swiftwasm Mar 13, 2020
MaxDesiatov pushed a commit that referenced this pull request Apr 21, 2020
Summary:
Previously `AtosSymbolizer` would set the PID to examine in the
constructor which is called early on during sanitizer init. This can
lead to incorrect behaviour in the case of a fork() because if the
symbolizer is launched in the child it will be told examine the parent
process rather than the child.

To fix this the PID is determined just before the symbolizer is
launched.

A test case is included that triggers the buggy behaviour that existed
prior to this patch. The test observes the PID that `atos` was called
on. It also examines the symbolized stacktrace. Prior to this patch
`atos` failed to symbolize the stacktrace giving output that looked
like...

```
  #0 0x100fc3bb5 in __sanitizer_print_stack_trace asan_stack.cpp:86
  #1 0x10490dd36 in PrintStack+0x56 (/path/to/print-stack-trace-in-code-loaded-after-fork.cpp.tmp_shared_lib.dylib:x86_64+0xd36)
  #2 0x100f6f986 in main+0x4a6 (/path/to/print-stack-trace-in-code-loaded-after-fork.cpp.tmp_loader:x86_64+0x100001986)
  #3 0x7fff714f1cc8 in start+0x0 (/usr/lib/system/libdyld.dylib:x86_64+0x1acc8)
```

After this patch stackframes `#1` and `#2` are fully symbolized.

This patch is also a pre-requisite refactor for rdar://problem/58789439.

Reviewers: kubamracek, yln

Subscribers: #sanitizers, llvm-commits

Tags: #sanitizers

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

(cherry picked from commit 8efc3cc)
MaxDesiatov pushed a commit that referenced this pull request Sep 4, 2020
When `Target::GetEntryPointAddress()` calls `exe_module->GetObjectFile()->GetEntryPointAddress()`, and the returned
`entry_addr` is valid, it can immediately be returned.

However, just before that, an `llvm::Error` value has been setup, but in this case it is not consumed before returning, like is done further below in the function.

In https://bugs.freebsd.org/248745 we got a bug report for this, where a very simple test case aborts and dumps core:

```
* thread #1, name = 'testcase', stop reason = breakpoint 1.1
    frame #0: 0x00000000002018d4 testcase`main(argc=1, argv=0x00007fffffffea18) at testcase.c:3:5
   1	int main(int argc, char *argv[])
   2	{
-> 3	    return 0;
   4	}
(lldb) p argc
Program aborted due to an unhandled Error:
Error value was Success. (Note: Success values must still be checked prior to being destroyed).

Thread 1 received signal SIGABRT, Aborted.
thr_kill () at thr_kill.S:3
3	thr_kill.S: No such file or directory.
(gdb) bt
#0  thr_kill () at thr_kill.S:3
#1  0x00000008049a0004 in __raise (s=6) at /usr/src/lib/libc/gen/raise.c:52
#2  0x0000000804916229 in abort () at /usr/src/lib/libc/stdlib/abort.c:67
#3  0x000000000451b5f5 in fatalUncheckedError () at /usr/src/contrib/llvm-project/llvm/lib/Support/Error.cpp:112
#4  0x00000000019cf008 in GetEntryPointAddress () at /usr/src/contrib/llvm-project/llvm/include/llvm/Support/Error.h:267
#5  0x0000000001bccbd8 in ConstructorSetup () at /usr/src/contrib/llvm-project/lldb/source/Target/ThreadPlanCallFunction.cpp:67
#6  0x0000000001bcd2c0 in ThreadPlanCallFunction () at /usr/src/contrib/llvm-project/lldb/source/Target/ThreadPlanCallFunction.cpp:114
#7  0x00000000020076d4 in InferiorCallMmap () at /usr/src/contrib/llvm-project/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:97
#8  0x0000000001f4be33 in DoAllocateMemory () at /usr/src/contrib/llvm-project/lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp:604
#9  0x0000000001fe51b9 in AllocatePage () at /usr/src/contrib/llvm-project/lldb/source/Target/Memory.cpp:347
#10 0x0000000001fe5385 in AllocateMemory () at /usr/src/contrib/llvm-project/lldb/source/Target/Memory.cpp:383
#11 0x0000000001974da2 in AllocateMemory () at /usr/src/contrib/llvm-project/lldb/source/Target/Process.cpp:2301
#12 CanJIT () at /usr/src/contrib/llvm-project/lldb/source/Target/Process.cpp:2331
#13 0x0000000001a1bf3d in Evaluate () at /usr/src/contrib/llvm-project/lldb/source/Expression/UserExpression.cpp:190
#14 0x00000000019ce7a2 in EvaluateExpression () at /usr/src/contrib/llvm-project/lldb/source/Target/Target.cpp:2372
#15 0x0000000001ad784c in EvaluateExpression () at /usr/src/contrib/llvm-project/lldb/source/Commands/CommandObjectExpression.cpp:414
#16 0x0000000001ad86ae in DoExecute () at /usr/src/contrib/llvm-project/lldb/source/Commands/CommandObjectExpression.cpp:646
#17 0x0000000001a5e3ed in Execute () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandObject.cpp:1003
#18 0x0000000001a6c4a3 in HandleCommand () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:1762
#19 0x0000000001a6f98c in IOHandlerInputComplete () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:2760
#20 0x0000000001a90b08 in Run () at /usr/src/contrib/llvm-project/lldb/source/Core/IOHandler.cpp:548
#21 0x00000000019a6c6a in ExecuteIOHandlers () at /usr/src/contrib/llvm-project/lldb/source/Core/Debugger.cpp:903
#22 0x0000000001a70337 in RunCommandInterpreter () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:2946
#23 0x0000000001d9d812 in RunCommandInterpreter () at /usr/src/contrib/llvm-project/lldb/source/API/SBDebugger.cpp:1169
#24 0x0000000001918be8 in MainLoop () at /usr/src/contrib/llvm-project/lldb/tools/driver/Driver.cpp:675
#25 0x000000000191a114 in main () at /usr/src/contrib/llvm-project/lldb/tools/driver/Driver.cpp:890```

Fix the incorrect error catch by only instantiating an `Error` object if it is necessary.

Reviewed By: JDevlieghere

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

(cherry picked from commit 1ce07cd)
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

Successfully merging this pull request may close these issues.

2 participants