Skip to content

Commit cf3bc44

Browse files
committed
fix: branch review fallout (toolchain, probe, dyld, alias)
1 parent f04e6c8 commit cf3bc44

6 files changed

Lines changed: 381 additions & 185 deletions

File tree

backend.go

Lines changed: 138 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ package main
1414
// equivalent path through the LLVM tools.
1515

1616
import (
17+
"crypto/sha256"
18+
"encoding/hex"
1719
"fmt"
1820
"os"
1921
"os/exec"
@@ -176,12 +178,14 @@ func forceBuildIDSha1(args []string) []string {
176178
// ============================================================
177179

178180
type lldArgvKey struct {
179-
triple string
180-
optLevel string
181-
debug bool
182-
ltoMode string
183-
macOS bool
184-
useLld bool
181+
triple string
182+
optLevel string
183+
debug bool
184+
ltoMode string
185+
macOS bool
186+
useLld bool
187+
standaloneDbg bool // -fstandalone-debug (darwin debug builds)
188+
extraHash string // sha256(extraCFlags) so e.g. -fsanitize=address gets its own probe slot
185189
}
186190

187191
type lldArgvEntry struct {
@@ -203,32 +207,63 @@ var (
203207
// machine, not per tin invocation.
204208
func lldArgvFor(opts linkOpts) *lldArgvEntry {
205209
key := lldArgvKey{
206-
triple: tripleFromTargetGOOS(opts.targetGOOS, opts.targetFlags),
207-
optLevel: opts.optLevel,
208-
debug: opts.debug,
209-
ltoMode: opts.ltoMode,
210-
macOS: opts.targetGOOS == "darwin",
211-
useLld: opts.useLld,
210+
triple: tripleFromTargetGOOS(opts.targetGOOS, opts.targetFlags),
211+
optLevel: opts.optLevel,
212+
debug: opts.debug,
213+
ltoMode: opts.ltoMode,
214+
macOS: opts.targetGOOS == "darwin",
215+
useLld: opts.useLld,
216+
standaloneDbg: opts.standaloneDebugMacOS,
217+
extraHash: hashExtraCFlags(opts.extraCFlags),
212218
}
213219

214220
if entry, ok := lookupLldArgv(key); ok {
215221
return entry
216222
}
217223

218-
if disk := readLldArgvFromDisk(key); disk != nil {
219-
storeLldArgv(key, disk)
224+
// Per-key lock so concurrent goroutines hitting the same key only
225+
// run the probe once (the second arrival re-checks the cache after
226+
// acquiring the lock and finds the first arrival's result).
227+
probeOnce(key).Do(func() {
228+
if disk := readLldArgvFromDisk(key); disk != nil {
229+
storeLldArgv(key, disk)
220230

221-
return disk
231+
return
232+
}
233+
234+
entry := probeLldArgv(opts)
235+
if entry.err == nil {
236+
writeLldArgvToDisk(key, entry)
237+
}
238+
239+
storeLldArgv(key, entry)
240+
})
241+
242+
if entry, ok := lookupLldArgv(key); ok {
243+
return entry
222244
}
223245

224-
entry := probeLldArgv(opts)
225-
if entry.err == nil {
226-
writeLldArgvToDisk(key, entry)
246+
return &lldArgvEntry{err: fmt.Errorf("lld probe: lookup miss after probe (internal)")}
247+
}
248+
249+
// hashExtraCFlags returns a stable short hash of the user-supplied
250+
// --cflag values so two link targets with different extras (e.g.
251+
// -fsanitize=address vs nothing) get distinct probe cache slots. The
252+
// link argv depends on these flags (ASan adds libasan to the link
253+
// line); without this in the key, a second link with different extras
254+
// would silently reuse the wrong cached argv.
255+
func hashExtraCFlags(flags []string) string {
256+
if len(flags) == 0 {
257+
return ""
227258
}
228259

229-
storeLldArgv(key, entry)
260+
h := sha256.New()
261+
for _, f := range flags {
262+
h.Write([]byte(f))
263+
h.Write([]byte{0})
264+
}
230265

231-
return entry
266+
return hex.EncodeToString(h.Sum(nil))[:16]
232267
}
233268

234269
func lookupLldArgv(key lldArgvKey) (*lldArgvEntry, bool) {
@@ -247,6 +282,29 @@ func storeLldArgv(key lldArgvKey, entry *lldArgvEntry) {
247282
lldArgvCache[key] = entry
248283
}
249284

285+
// probeOnce returns the per-key sync.Once used to coalesce concurrent
286+
// probes for the same lookup key. The Once value is keyed by the full
287+
// lldArgvKey so distinct (target, opt-level, ...) combos still probe
288+
// in parallel; only redundant probes for the SAME key block.
289+
var (
290+
probeOnceMap = map[lldArgvKey]*sync.Once{}
291+
probeOnceMu sync.Mutex
292+
)
293+
294+
func probeOnce(key lldArgvKey) *sync.Once {
295+
probeOnceMu.Lock()
296+
defer probeOnceMu.Unlock()
297+
298+
if o, ok := probeOnceMap[key]; ok {
299+
return o
300+
}
301+
302+
o := &sync.Once{}
303+
probeOnceMap[key] = o
304+
305+
return o
306+
}
307+
250308
// lldProbeCacheDir is the on-disk cache root for linker-argv probe
251309
// results. Lives under .build/host-info alongside the host clang
252310
// metadata so a single `rm -rf .build/host-info` clears every host-
@@ -355,6 +413,14 @@ func probeLldArgv(opts linkOpts) *lldArgvEntry {
355413
args = append(args, "-flto=thin")
356414
}
357415

416+
if opts.debug {
417+
args = append(args, "-g")
418+
}
419+
420+
if opts.standaloneDebugMacOS && opts.targetGOOS == "darwin" {
421+
args = append(args, "-fstandalone-debug")
422+
}
423+
358424
args = append(args, opts.targetFlags...)
359425

360426
if opts.functionSecs {
@@ -377,6 +443,13 @@ func probeLldArgv(opts linkOpts) *lldArgvEntry {
377443
args = append(args, "-fuse-ld=lld")
378444
}
379445

446+
// extraCFlags is the user's --cflag passthrough. Some entries
447+
// (e.g. -fsanitize=address) inject runtime libraries into the
448+
// link line, so they MUST be in the probe argv -- otherwise the
449+
// cached result would omit those libs and the link command tin
450+
// later issues would fail to find them.
451+
args = append(args, opts.extraCFlags...)
452+
380453
args = append(args, tmpPath, "-o", dummyOutPath, "-###")
381454
out, _ := exec.Command("clang", args...).CombinedOutput()
382455

@@ -387,10 +460,7 @@ func probeLldArgv(opts linkOpts) *lldArgvEntry {
387460

388461
last := lines[len(lines)-1]
389462

390-
toks, err := splitShellLine(last)
391-
if err != nil {
392-
return &lldArgvEntry{err: fmt.Errorf("lld probe: shlex: %w", err)}
393-
}
463+
toks := splitShellLine(last)
394464

395465
if len(toks) == 0 {
396466
return &lldArgvEntry{err: fmt.Errorf("lld probe: empty argv")}
@@ -480,7 +550,11 @@ func writeDummyBitcode(path string, opts linkOpts) error {
480550

481551
// splitShellLine parses a shell-style command line into argv tokens,
482552
// respecting double quotes (clang's -### uses them for every arg).
483-
func splitShellLine(line string) ([]string, error) {
553+
// Inside a quoted run, only `\\` and `\"` are recognized as escape
554+
// sequences (they unwrap to `\` and `"` respectively); a stray `\`
555+
// followed by anything else stays literal. Outside quotes a `\\` quotes
556+
// the next character verbatim.
557+
func splitShellLine(line string) []string {
484558
var (
485559
toks []string
486560
buf strings.Builder
@@ -489,6 +563,37 @@ func splitShellLine(line string) ([]string, error) {
489563

490564
for i := 0; i < len(line); i++ {
491565
c := line[i]
566+
567+
// Escape sequences: handle them BEFORE the quote toggle so
568+
// `\"` inside a quoted span stays a literal quote rather than
569+
// closing the run.
570+
if c == '\\' && i+1 < len(line) {
571+
next := line[i+1]
572+
573+
if in {
574+
if next == '"' || next == '\\' {
575+
i++
576+
577+
buf.WriteByte(next)
578+
579+
continue
580+
}
581+
// Unknown escape inside quotes: keep both bytes
582+
// literal (matches POSIX shell behavior for
583+
// double-quoted strings).
584+
buf.WriteByte(c)
585+
586+
continue
587+
}
588+
589+
// Outside quotes: unconditional escape of the next byte.
590+
i++
591+
592+
buf.WriteByte(next)
593+
594+
continue
595+
}
596+
492597
if c == '"' {
493598
in = !in
494599

@@ -504,26 +609,24 @@ func splitShellLine(line string) ([]string, error) {
504609
continue
505610
}
506611

507-
if c == '\\' && i+1 < len(line) {
508-
i++
509-
buf.WriteByte(line[i])
510-
511-
continue
512-
}
513-
514612
buf.WriteByte(c)
515613
}
516614

517615
if buf.Len() > 0 {
518616
toks = append(toks, buf.String())
519617
}
520618

521-
return toks, nil
619+
return toks
522620
}
523621

524622
// isClangTempForBase reports whether t looks like clang's renamed
525623
// temp object for an input whose basename (without extension) equals
526-
// `base`. Clang produces `/tmp/<base>-<hex>.o` under -flto=thin.
624+
// `base`. Clang produces `/tmp/<base>-<hex>.o` under -flto=thin where
625+
// the hex run is always at least six characters (`mkstemp`-style).
626+
// We require the same minimum so a foreign `.o` named `dummy-1.o`
627+
// can't be mistaken for the dummy.
628+
const clangTempMinHex = 6
629+
527630
func isClangTempForBase(t, base string) bool {
528631
if !strings.HasSuffix(t, ".o") {
529632
return false
@@ -535,7 +638,7 @@ func isClangTempForBase(t, base string) bool {
535638
}
536639

537640
rest := strings.TrimSuffix(strings.TrimPrefix(bt, base+"-"), ".o")
538-
if len(rest) == 0 {
641+
if len(rest) < clangTempMinHex {
539642
return false
540643
}
541644

codegen/exprs_ops.go

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,14 +1145,28 @@ func (cg *CodeGen) genTupleLit(block *ir.Block, tup *ast.TupleLit, expectedType
11451145
}
11461146
}
11471147

1148-
// genTypeDecl may have registered concreteName as a typeAlias to a
1149-
// canonical-key form (e.g. `Tuple__udp::Conn__*errors::Error` ->
1150-
// `Tuple__udp__Conn__*errors__Error`); follow the alias chain before
1151-
// looking up the struct type.
1152-
if alias, ok := cg.typeAliases[concreteName]; ok {
1153-
if st, isSimple := alias.(*ast.SimpleType); isSimple {
1154-
concreteName = st.Name
1148+
// genTypeDecl may have registered concreteName as a typeAlias to
1149+
// a canonical-key form (e.g. `Tuple__udp::Conn__*errors::Error`
1150+
// -> `Tuple__udp__Conn__*errors__Error`). Follow the alias chain
1151+
// (with a safety bound) before looking up the struct type so
1152+
// multi-hop aliases also resolve. The 64-step cap mirrors
1153+
// typeExprCanonicalKeyN's recursion guard.
1154+
for i := 0; i < 64; i++ {
1155+
alias, ok := cg.typeAliases[concreteName]
1156+
if !ok {
1157+
break
1158+
}
1159+
1160+
st, isSimple := alias.(*ast.SimpleType)
1161+
if !isSimple {
1162+
break
11551163
}
1164+
1165+
if st.Name == concreteName {
1166+
break
1167+
}
1168+
1169+
concreteName = st.Name
11561170
}
11571171

11581172
st, ok := cg.structTypes[concreteName]

0 commit comments

Comments
 (0)