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

[SR-10994] default arguments broken in LLDB REPL #4483

Open
marcrasi mannequin opened this issue Jun 21, 2019 · 17 comments
Open

[SR-10994] default arguments broken in LLDB REPL #4483

marcrasi mannequin opened this issue Jun 21, 2019 · 17 comments
Labels
bug Something isn't working LLDB for Swift

Comments

@marcrasi
Copy link
Mannequin

marcrasi mannequin commented Jun 21, 2019

Previous ID SR-10994
Radar None
Original Reporter @marcrasi
Type Bug
Additional Detail from JIRA
Votes 0
Component/s LLDB for Swift
Labels Bug
Assignee None
Priority Medium

md5: efd3e2805e709b6935327dc6f2ce1e32

relates to:

  • TF-587 Default arguments not working in Swift-Jupyter

Issue Description:

On Ubuntu 18.04, swift development snapshot 2019-06-20:

saeta@saeta-v04-jupyter:~/tmp$ ./swift-DEVELOPMENT-SNAPSHOT-2019-06-20-a-ubuntu18.04/usr/bin/swift
Welcome to Swift version 5.1-dev (LLVM af1f73e9e9, Swift 543632deda).
Type :help for assistance.
  1> func foo(_ a: String, b: String? = nil) { print(a) }
  2> foo("asdf")
error: Couldn't lookup symbols:
  default argument 1 of __lldb_expr_1.foo(_: Swift.String, b: Swift.Optional<Swift.String>) -> ()

This error does not occur on swift-5.0.1-RELEASE-ubuntu18.04.

I can fix it by reverting apple/swift@846a64b538145ba70bd76185a080876a34c93991, but I don't know enough about linkage to understand what a proper fix would be.

@belkadan
Copy link

cc @compnerd, @dcci

@dcci
Copy link
Mannequin

dcci mannequin commented Jun 24, 2019

Looking.

@dcci
Copy link
Mannequin

dcci mannequin commented Jun 25, 2019

Can reproduce this on macOS too

  1> func foo(_ a: String, b: String? = nil) { print(a) }
  2> foo("asdf")
error: Couldn't lookup symbols:
  default argument 1 of __lldb_expr_1.foo(_: Swift.String, b: Swift.Optional<Swift.String>) -> ()

@compnerd
Copy link
Collaborator

Hmm, I think that it seems like an IRGen issue where we don’t generate the default value thunk and mark it for emission ({{@llvm.compiler.used}}).

@compnerd
Copy link
Collaborator

; ModuleID = 'lldb_module'
source_filename = "lldb_module"
target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-pc-windows-msvc19.21.27702"
%swift.bridge.109 = type opaque
$"$sSSSgWOe" = comdat any
@0 = private unnamed_addr constant [2 x i8] c"s\00"
define dllexport i32 @main(i32, i8**) #0 21 {
%3 = bitcast i8** %1 to i8*
br label %4, 28
; <label>:4: ; preds = %2
%5 = call swiftcc { i64, %swift.bridge.109* } @"$sSS21_builtinStringLiteral17utf8CodeUnitCount7isASCIISSBp_BwBi1_tcfC"(i8* getelementptr inbounds ([2 x i8], [2 x i8]* @0, i64 0, i64 0), i64 1, i1 true), 31
%6 = extractvalue { i64, %swift.bridge.109* } %5, 0, 31
%7 = extractvalue { i64, %swift.bridge.109* } %5, 1, 31
%8 = call swiftcc { i64, i64 } @"$s13_lldb_expr_13foo_1bySS_SSSgtFfA0"(), 33
%9 = extractvalue { i64, i64 } %8, 0, 33
%10 = extractvalue { i64, i64 } %8, 1, 33
call swiftcc void @"$s13__lldb_expr_13foo_1bySS_SSSgtF"(i64 %6, %swift.bridge.109* %7, i64 %9, i64 %10), 34
call void @"$sSSSgWOe"(i64 %9, i64 %10), 34
call void @swift_bridgeObjectRelease(%swift.bridge.109* %7) #3, 34
br label %11, 28
; <label>:11: ; preds = %4
ret i32 0, 28
}
declare dllimport swiftcc { i64, %swift.bridge.109* } @"$sSS21_builtinStringLiteral17utf8CodeUnitCount7isASCIISSBp_BwBi1_tcfC"(i8*, i64, i1) #0
; Function Attrs: cold noreturn nounwind
declare void @llvm.trap() #1
declare dllimport swiftcc { i64, i64 } @"$s13_lldb_expr_13foo_1bySS_SSSgtFfA0"() #0
declare dllimport swiftcc void @"$s13__lldb_expr_13foo_1bySS_SSSgtF"(i64, %swift.bridge.109*, i64, i64) #0
; Function Attrs: noinline nounwind
define linkonce_odr hidden void @"$sSSSgWOe"(i64, i64) #2 comdat {
%3 = icmp eq i64 %1, 0
br i1 %3, label %6, label %4
; <label>:4: ; preds = %2
%5 = inttoptr i64 %1 to %swift.bridge.109*
call void @swift_bridgeObjectRelease(%swift.bridge.109* %5) #3
br label %6
; <label>:6: ; preds = %4, %2
ret void
}
; Function Attrs: nounwind
declare dllimport void @swift_bridgeObjectRelease(%swift.bridge.109*) #3
attributes #0 = { "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" }
attributes #1 = { cold noreturn nounwind }
attributes #2 = { noinline nounwind "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" }
attributes #3 = { nounwind }
![](llvm.dbg.cu = ){11}
![](swift.module.flags = ){!13}
![](llvm.linker.options = ){15}
![](llvm.module.flags = ){17, 19, !20}
![](0 = distinct )DICompileUnit(language: DW_LANG_Swift, file: ![](1, producer: "Swift version 5.1-dev (LLVM af1f73e9e9, Swift 50b46f4d88)", isOptimized: false, runtimeVersion: 5, emissionKind: FullDebug, enums: )2, globals: ![](3, imports: )9)
![](1 = )DIFile(filename: "<REPL>", directory: "")
![](2 = ){}
![](3 = ){!4}
![](4 = )DIGlobalVariableExpression(var: ![](5, expr: )DIExpression(DW_OP_constu, 0, DW_OP_stack_value))
![](5 = distinct )DIGlobalVariable(name: "$__lldb_result", scope: ![](6, file: )7, type: !8, isLocal: false, isDefinition: true)
![](6 = )DIModule(scope: null, name: "__lldb_expr_8")
![](7 = )DIFile(filename: "<compiler-generated>", directory: "")
![](8 = )DICompositeType(tag: DW_TAG_structure_type, name: "$sytD", file: ![](1, elements: )2, runtimeLang: DW_LANG_Swift, identifier: "$sytD")
![](9 = ){!10}
![](10 = )DIImportedEntity(tag: DW_TAG_imported_module, scope: ![](1, entity: )6, file: !1)
![](11 = distinct )DICompileUnit(language: DW_LANG_C99, file: ![](12, producer: "clang version 8.0.0 (https://git.llvm.org/git/clang.git e6ad8a4a2747549545461a2d577acd67c25771fe) (https://github.com/apple/swift-llvm.git af1f73e9e916e297f5d224c64b1a185bad068f01)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: )2, nameTableKind: None)
![](12 = )DIFile(filename: "<swift-imported-modules>", directory: "S:\5Cb\5Clldb")
![](13 = ){!"standard-library", i1 false}
![](14 = ){!"/DEFAULTLIB:swiftCore.lib"}
![](15 = ){!"/DEFAULTLIB:legacy_stdio_definitions.lib"}
![](16 = ){i32 2, !"Debug Info Version", i32 3}
![](17 = ){i32 1, !"wchar_size", i32 2}
![](18 = ){i32 7, !"PIC Level", i32 2}
![](19 = ){i32 4, !"Objective-C Garbage Collection", i32 83953408}
![](20 = ){i32 1, !"Swift Version", i32 7}
![](21 = distinct )DISubprogram(name: "main", linkageName: "main", scope: ![](6, file: )1, line: 1, type: ![](22, spFlags: DISPFlagDefinition, unit: )0, retainedNodes: !2)
![](22 = )DISubroutineType(types: !23)
![](23 = ){24, !27}
![](24 = )DICompositeType(tag: DW_TAG_structure_type, name: "Int32", scope: ![](26, file: )25, size: 32, elements: !2, runtimeLang: DW_LANG_Swift, identifier: "$ss5Int32VD")
![](25 = )DIFile(filename: "S:\5Cb\5Clldb\5Clib\5Clldb\5Cswift\5Cwindows\5Cx86_64\5CSwift.swiftmodule", directory: "")
![](26 = )DIModule(scope: null, name: "Swift", includePath: "S:\5Cb\5Clldb\5Clib\5Clldb\5Cswift\5Cwindows\5Cx86_64\5CSwift.swiftmodule")
![](27 = )DICompositeType(tag: DW_TAG_structure_type, name: "UnsafeMutablePointer", scope: ![](26, file: )1, size: 64, elements: !2, runtimeLang: DW_LANG_Swift, identifier: "$sSpySpys4Int8VGSgGD")
![](28 = )DILocation(line: 2, column: 1, scope: !29)
![](29 = distinct )DILexicalBlock(scope: ![](21, file: )30, line: 2, column: 1)
![](30 = )DIFile(filename: "repl.swift", directory: "")
![](31 = )DILocation(line: 2, column: 5, scope: !32)
![](32 = distinct )DILexicalBlock(scope: ![](29, file: )30, line: 2, column: 1)
![](33 = )DILocation(line: 2, column: 4, scope: !32)
![](34 = )DILocation(line: 2, column: 1, scope: !32)

The interesting bit to follow here is `main`, which does the follow:

%8 = call swiftcc { i64, i64 } @"$s13_lldb_expr_13foo_1bySS_SSSgtFfA0"(), 33
call swiftcc void @"$s13__lldb_expr_13foo_1bySS_SSSgtF"(i64 %6, %swift.bridge.109* %7, i64 %9, i64 %10), 34

The problem is that $s13_lldb_expr_13foo_1bySS_SSSgtFfA0 is never defined. This is a bug in the lowering of the expression. The default argument construct thunk should have been materialised during the emission of the callee and placed into the global Swift module (unless the default value thunk is marked as @__inline(always), where in it would need to be materialised at the caller side). This would then invoke that default value generator thunk that was previously emitted into the module.

@compnerd
Copy link
Collaborator

Discussing this a bit with @dcci, I think that the following should help explain what needs to happen:

$ type S:\reduced.swift

public func f(_ s: String, _ t: String? = nil) { }

$ swiftc -emit-sil -o - -O S:\reduced.swift -module-name __lldb_expr
sil_stage canonical``import Builtin
import Swift
import SwiftShims``public func f(_ s: String, _ t: String? = nil)``// main
sil @main : $@convention(c) (Int32, UnsafeMutablePointer<Optional<UnsafeMutablePointer<Int8>>>) -> Int32 {
bb0(%0 : $Int32, %1 : $UnsafeMutablePointer<Optional<UnsafeMutablePointer<Int8>>>):
{{ %2 = integer_literal $Builtin.Int32, 0 // user: %3}}
{{ %3 = struct $Int32 (%2 : $Builtin.Int32) // user: %4}}
{{ return %3 : $Int32 // id: %4}}
} // end sil function 'main'``// default argument 1 of f(:🙂
sil non_abi @$s11_lldb_expr1fyySS_SSSgtFfA0 : $@convention(thin) () -> @owned Optional<String> {
bb0:
{{ %0 = enum $Optional<String>, #Optional.none!enumelt // user: %1}}
{{ return %0 : $Optional<String> // id: %1}}
} // end sil function '$s11_lldb_expr1fyySS_SSSgtFfA0'``// f(:🙂
sil @$s11__lldb_expr1fyySS_SSSgtF : $@convention(thin) (@guaranteed String, @guaranteed Optional<String>) -> () {
bb0(%0 : $String, %1 : $Optional<String>):
{{ %2 = tuple () // user: %3}}
{{ return %2 : $() // id: %3}}
} // end sil function '$s11__lldb_expr1fyySS_SSSgtF'

What this shows is the interesting stuff. In particular what is interesting here is:

$s11_lldb_expr1fyySS_SSSgtFfA0

That is the default value thunk. (You can verify this with swift demangle: default argument 1 of __lldb_expr.f(Swift.String, Swift.String?) -> ())

SILGen is generating it, the question is why is this not happening in the SwiftExpressionParser plugin in lldb.

@compnerd
Copy link
Collaborator

So, thanks to some help from @dcci - we have the log from lldb on macOS. The interesting thing to look at here is the SILGen output. It appears that the SILGen phase will generate the declaration not the definition of the default value thunk, which is why the body is never materialised and thus the linking fails.

@compnerd
Copy link
Collaborator

CC: @slavapestov

@slavapestov
Copy link
Member

The thunk has PublicNonABI linkage so I would not expect it to show in the generated IR when its owner function is emitted, unless something references it.

In the normal case, the default argument is serialized as part of the SIL for the module defining the function with the default argument. When you call the function we deserialize the default argument generator from the SIL module and emit it as part of the caller. I'm guessing we don't preserve SIL in this case in lldb. You can see if this is the case by trying the following:

@_alwaysEmitIntoClient func f() {}
f()

If you enter the two lines as separate REPL inputs it will probably show the same problem.

@marcrasi
Copy link
Mannequin Author

marcrasi mannequin commented Jun 26, 2019

Indeed LLDB does not preserve SIL for the lines that it compiles and executes. There was some discussion about LLDB preserving SIL here: https://forums.swift.org/t/generating-serialized-sil-for-inlinable-transparent-functions-in-repl-expressions/16237?u=marcrasi

@dcci
Copy link
Mannequin

dcci mannequin commented Jun 26, 2019

FWIW, the other example works:

 % ./lldb --repl
Welcome to Swift version 5.1-dev (LLVM af1f73e9e9, Swift b2ce4a2ba1).
Type :help for assistance.
  1> @_alwaysEmitIntoClient func f() {} 
  2> f()

@slavapestov
Copy link
Member

What if 'f' is public?

So I thought about it some more and my explanation of what happens in the non-REPL case is not completely accurate. When building without whole module optimization, we can reference default arguments from other translation units, and we don't have serialized SIL (since each TU is built in parallel). So they should be emitted with hidden linkage. Does the REPL think it's doing "whole module" build?

@dcci
Copy link
Mannequin

dcci mannequin commented Jun 26, 2019

Making `f` public doesn't seem to change anything.

 % ./lldb --repl
Welcome to Swift version 5.1-dev (LLVM af1f73e9e9, Swift b2ce4a2ba1).
Type :help for assistance.
  1> @_alwaysEmitIntoClient public func f() {}
  2> f()
  3>  

I don't think the REPL is doing whole module build, I would be surprised, but you never know. Is there a quick way to check? [e.g. the compiler invocation has some flag set?]

@compnerd
Copy link
Collaborator

The given example is uninteresting as there are no default value thunks involved. @slavapestov, actually, the SIL is preserved.

I had a bit of an epiphany last night, drifting off to sleep: the problem is not in the SILGen or IRGen, but rather in the implementation of LLDB, and in particular, LLDB's expression JIT handling.

Consider the following abbreviated log from lldb:

> func f(_ s: String, _ t: String? = nil) { }

This generates the following SIL:

sil_stage raw

import Builtin
import Swift
import SwiftShims

func f(_ s: String, _ t: String? = nil)

// main
sil [ossa] @main : $@convention(c) (Int32, UnsafeMutablePointer<Optional<UnsafeMutablePointer<Int8>>>) -> Int32 {
bb0(%0 : $Int32, %1 : $UnsafeMutablePointer<Optional<UnsafeMutablePointer<Int8>>>):
%2 = integer_literal $Builtin.Int32, 0 // user: %3
%3 = struct $Int32 (%2 : $Builtin.Int32) // user: %4
return %3 : $Int32 // id: %4
} // end sil function 'main'

// default argument 1 of f(*:*🙂
sil non_abi [serialized] [ossa] @$s13_lldb_expr_11fyySS_SSSgtFfA0 : $@convention(thin) () -> @owned Optional<String> {
bb0:
%0 = enum $Optional<String>, #Optional.none!enumelt // user: %1
return %0 : $Optional<String> // id: %1
} // end sil function '$s13_lldb_expr_11fyySS_SSSgtFfA0'

// f(*:*🙂
sil [ossa] @$s13__lldb_expr_11fyySS_SSSgtF : $@convention(thin) (@guaranteed String, @guaranteed Optional<String>) -> () {
// %0 // user: %2
// %1 // user: %3
bb0(%0 : @guaranteed $String, %1 : @guaranteed $Optional<String>):
debug_value %0 : $String, let, name "s", argno 1 // id: %2
debug_value %1 : $Optional<String>, let, name "t", argno 2 // id: %3
%4 = tuple () // user: %5
return %4 : $() // id: %5
} // end sil function '$s13__lldb_expr_11fyySS_SSSgtF'

Note that the default argument thunk is emitted.

It is then lowered in the following IR:

define weak_odr hidden swiftcc { i64, i64 } @"$s13_lldb_expr_11fyySS_SSSgtFfA0"() #0 comdat 28 {
ret { i64, i64 } zeroinitializer, 33
}

define dllexport swiftcc void @"$s13__lldb_expr_11fyySS_SSSgtF"(i64, %swift.bridge*, i64, i64) #0 34 {
%5 = alloca %TSS, align 8
call void @llvm.dbg.declare(metadata %TSS* %5, metadata ![](39, metadata )DIExpression()), 40
%6 = bitcast %TSS* %5 to i8*
call void @llvm.memset.p0i8.i64(i8* align 8 %6, i8 0, i64 16, i1 false)
%7 = alloca %TSSSg, align 8
call void @llvm.dbg.declare(metadata %TSSSg* %7, metadata ![](41, metadata )DIExpression()), 42
%8 = bitcast %TSSSg* %7 to i8*
call void @llvm.memset.p0i8.i64(i8* align 8 %8, i8 0, i64 16, i1 false)
%9 = bitcast %TSS* %5 to i8*, 43
call void @llvm.lifetime.start.p0i8(i64 16, i8* %9), 43
%10 = getelementptr inbounds %TSS, %TSS* %5, i32 0, i32 0, 45
%11 = getelementptr inbounds %Ts11_StringGutsV, %Ts11_StringGutsV* %10, i32 0, i32 0, 45
%12 = getelementptr inbounds %Ts13_StringObjectV, %Ts13_StringObjectV* %11, i32 0, i32 0, 45
%13 = getelementptr inbounds %Ts6UInt64V, %Ts6UInt64V* %12, i32 0, i32 0, 45
store i64 %0, i64* %13, align 8, 45
%14 = getelementptr inbounds %Ts13_StringObjectV, %Ts13_StringObjectV* %11, i32 0, i32 1, 45
store %swift.bridge* %1, %swift.bridge** %14, align 8, 45
%15 = bitcast %TSSSg* %7 to i8*, 43
call void @llvm.lifetime.start.p0i8(i64 16, i8* %15), 43
%16 = bitcast %TSSSg* %7 to { i64, i64 }*, 45
%17 = getelementptr inbounds { i64, i64 }, { i64, i64 }* %16, i32 0, i32 0, 45
store i64 %2, i64* %17, align 8, 45
%18 = getelementptr inbounds { i64, i64 }, { i64, i64 }* %16, i32 0, i32 1, 45
store i64 %3, i64* %18, align 8, 45
ret void, 46
}

Furthermore, it appears that it is even materialized and emitted into the module.

And, now, the pièce de résistance

> f("f")

error: Couldn't lookup symbols:
default argument 1 of __lldb_expr_1.f(Swift.String, Swift.Optional<Swift.String>) -> ()

Lets take a closer look at the IRGen for the `main` expression:

define dllexport i32 @main(i32, i8**) #0 21 {
%3 = bitcast i8** %1 to i8*
br label %4, 28

; <label>:4: ; preds = %2
%5 = call swiftcc { i64, %swift.bridge.25* } @"$sSS21_builtinStringLiteral17utf8CodeUnitCount7isASCIISSBp_BwBi1_tcfC"(i8* getelementptr inbounds ([2 x i8], [2 x i8]* @0, i64 0, i64 0), i64 1, i1 true), 31
%6 = extractvalue { i64, %swift.bridge.25* } %5, 0, 31
%7 = extractvalue { i64, %swift.bridge.25* } %5, 1, 31
%8 = call swiftcc { i64, i64 } @"$s13_lldb_expr_11fyySS_SSSgtFfA0"(), 33
%9 = extractvalue { i64, i64 } %8, 0, 33
%10 = extractvalue { i64, i64 } %8, 1, 33
call swiftcc void @"$s13__lldb_expr_11fyySS_SSSgtF"(i64 %6, %swift.bridge.25* %7, i64 %9, i64 %10), 34
call void @"$sSSSgWOe"(i64 %9, i64 %10), 34
call void @swift_bridgeObjectRelease(%swift.bridge.25* %7) #3, 34
br label %11, 28

; <label>:11: ; preds = %4
ret i32 0, 28
}

We are interested in the def of value 8:

%8 = call swiftcc { i64, i64 } @"$s13_lldb_expr_11fyySS_SSSgtFfA0"(), 33

So, the call is to the proper function. Why then does this fail? We have materialised the value, the name of the symbols are the same, so the JIT should be able to resolve the symbol. And the answer to that lies burried in the mess of lldb's output:

When we initially declared the function:

Registering JITted Functions:

Function: $s13__lldb_expr_11fyySS_SSSgtF at 0x1b0a7fa0030.
Registering JIIted Symbols:

000002F1DEC778F0 ObjectFile::ObjectFile() module = 000002F1E8CD4F70 (), file = <NULL>, file_offset = 0x00000000, size = 0

And when we finally invoked the function:

Registering JITted Functions:

Registering JIIted Symbols:

000002F1DEC793F0 ObjectFile::ObjectFile() module = 000002F1E8CD5630 (), file = <NULL>, file_offset = 0x00000000, size = 0
Breakpoint::ModulesChanged: num_modules: 1 load: 1 delete_locations: 0

There are two interesting things to realize here:

  1. the default value thunk function was never registered with the JIT
  2. each invocation is in a different module

The key thing to realize here is that because the symbols are in a different module, the lookup will fail in MCJIT. This means that the call will bubble up to the dyld handler, which I believe is PersistentExpressionState::LookupSymbol which will then consult m_symbol_map for the address of the function. However, we failed to register the thunk with the persistent state in the first place so we have no record of the symbol leading us to the unresolved symbol.

@compnerd
Copy link
Collaborator

So, solving this is a bit more complicated. The information that we need is only in the AST. However, the AST does not have the declarations that are needed as they are materialized only during SILGen. Furthermore, the definitions which need to be preserved are only available after IRGen.

@slavapestov
Copy link
Member

> The given example is uninteresting as there are no default value thunks involved.

A public @_alwaysEmitIntoClient function should behave identically to a default argument generator for a public function.

@compnerd
Copy link
Collaborator

Oh, interesting. Didn't know that impact of the @_alwaysEmitIntoClient ... I suppose that makes sense that it would have to be PublicNonABI for that to work properly since you could run against a different binary, which should give it the same linkage type. However, that still would get tracked as it is the **symbol being defined. The problem here is that lldb does not track auxiliary symbols for the main symbol. That is, the default valued argument thunks are special since they need to be preserved with the actual function.

Come to think of it, we could/should just emit the default value thunks into an ELF group (SHF_GROUP) or a COFF COMDAT. That would ensure that if any of the symbols are preserved then the entire group is preserved. If the default value thunk is emitted but not the function, no group is necessary. The one way that this can break down is if we alias the default value thunk across multiple functions though since I can see ICF occurring over simple thunks such as String? = nil.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working LLDB for Swift
Projects
None yet
Development

No branches or pull requests

3 participants