Symbol extractor: handle more cases. This adds support for default arguments, and fixes handling of interaction between `extern "C"` and anonymous namespaces. Also, add some debug info to the output to help with symbol prefixing work: - Source file - Source line - Source byte range (not including function body) - Names of function arguments Bug: 42220000 Change-Id: If3b9795207912391781dc8e454533823b3051c15 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/85530 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Rudolf Polzer <rpolzer@google.com> Auto-Submit: Rudolf Polzer <rpolzer@google.com>
diff --git a/util/extract_identifiers_clang_json.go b/util/extract_identifiers_clang_json.go index 37fc5d9..ec75c1d 100644 --- a/util/extract_identifiers_clang_json.go +++ b/util/extract_identifiers_clang_json.go
@@ -33,6 +33,7 @@ "fmt" "log" "os" + "path" "regexp" "strings" ) @@ -48,11 +49,12 @@ type node struct { Kind string Loc loc - Inner []*node `json:",omitempty"` - Decl *node + Range rangeStruct `json:",omitempty"` + Inner []*node `json:",omitempty"` // Node fields that may or may not matter depending on `Kind`. CompleteDefinition bool `json:",omitempty"` + Decl *node `json:",omitempty"` IsImplicit bool `json:",omitempty"` Language string `json:",omitempty"` Name string `json:",omitempty"` @@ -61,49 +63,76 @@ TagUsed string `json:",omitempty"` } +// rangeStruct is a location range from the Clang AST dump. +type rangeStruct struct { + Begin *loc `json:",omitempty"` + End *loc `json:",omitempty"` +} + // loc is a location from the Clang AST dump. type loc struct { File string `json:",omitempty"` + Line uint `json:",omitempty"` + Col uint `json:",omitempty"` + Offset uint `json:",omitempty"` + TokLen uint `json:",omitempty"` SpellingLoc *loc `json:",omitempty"` ExpansionLoc *loc `json:",omitempty"` } -// file finds the file path of a loc. -func (l loc) file() string { +// expansionLoc returns the expansion location of the given loc. +func (l loc) expansionLoc() loc { if l.ExpansionLoc != nil { - return l.ExpansionLoc.file() + return *l.ExpansionLoc } if l.SpellingLoc != nil { - return l.SpellingLoc.file() + return *l.SpellingLoc } - return l.File + return l +} + +type decompressCtx struct { + file string + line uint } // decompress undoes the filename field compression from // JSONNodeDumper::writeSourceLocation and JSONNodeDumper::writeBareSourceLocation. -func (l *loc) decompress(lastFile *string) { +func (l *loc) decompress(last *decompressCtx) { if l == nil { return } - l.SpellingLoc.decompress(lastFile) - l.ExpansionLoc.decompress(lastFile) + l.SpellingLoc.decompress(last) + l.ExpansionLoc.decompress(last) if l.SpellingLoc != nil || l.ExpansionLoc != nil { return } if l.File == "" { - l.File = *lastFile + l.File = last.file } else { - *lastFile = l.File + last.file = l.File } + if l.Line == 0 { + l.Line = last.line + } else { + last.line = l.Line + } +} + +// path returns the loc's file path in clean, unique form. +func (l loc) path() string { + return path.Clean(l.File) } // decompressLocsInternal is a helper for decompressLocs. // // It keeps state in its lastFile pointer. -func (n *node) decompressLocsInternal(lastFile *string) { - n.Loc.decompress(lastFile) +func (n *node) decompressLocsInternal(last *decompressCtx) { + n.Loc.decompress(last) + n.Range.Begin.decompress(last) + n.Range.End.decompress(last) for _, child := range n.Inner { - child.decompressLocsInternal(lastFile) + child.decompressLocsInternal(last) } } @@ -111,8 +140,7 @@ // // Should be called right after parsing. func (n *node) decompressLocs() { - var lastFile string - n.decompressLocsInternal(&lastFile) + n.decompressLocsInternal(&decompressCtx{}) } // storage represents the storage class of a node. @@ -138,6 +166,48 @@ return storage, nil } +// functionArgs returns all the function arg nodes of a node. +func (n node) functionArgs() []*node { + var result []*node + for _, child := range n.Inner { + if child.Kind == "ParmVarDecl" { + result = append(result, child) + } + } + return result +} + +func (n node) locationRange() (file string, line, start, end uint, ok bool) { + from := n.Range.Begin.expansionLoc() + to := n.Range.End.expansionLoc() + if from.path() != to.path() { + return "", 0, 0, 0, false + } + return from.path(), from.Line, from.Offset, to.Offset + to.TokLen, true +} + +func (n node) locationRangeWithoutBody() (file string, line, start, end uint, ok bool) { + file, line, start, end, ok = n.locationRange() + if !ok { + return + } + // Cut bad nodes from the end. + for i := len(n.Inner) - 1; i >= 0; i-- { + child := n.Inner[i] + if child.Kind != "CompoundStmt" { + continue + } + cfile, _, cstart, cend, ok := child.locationRange() + if !ok { + continue + } + if cfile == file && cend == end { + end = cstart + } + } + return +} + // namespacing indicates how the identifier respects namespaces. type namespacing int @@ -200,7 +270,7 @@ w.inBoringSSL = true return } - w.inBoringSSL = boringSSLPath.MatchString(loc.file()) + w.inBoringSSL = boringSSLPath.MatchString(loc.expansionLoc().path()) } // visit traverses a node in the AST and analyzes it for identifiers contained therein. @@ -248,7 +318,7 @@ return nil } if n.Name != "" { - if err := w.collectIdentifier(n.TagUsed, alwaysNamespaced, neverLinked, noStorage, n.Name); err != nil { + if err := w.collectIdentifier(n, n.TagUsed, alwaysNamespaced, neverLinked, noStorage); err != nil { return err } } @@ -258,7 +328,7 @@ return nil } if n.Name != "" { - if err := w.collectIdentifier("enum", alwaysNamespaced, neverLinked, noStorage, n.Name); err != nil { + if err := w.collectIdentifier(n, "enum", alwaysNamespaced, neverLinked, noStorage); err != nil { return err } } @@ -266,7 +336,7 @@ if w.record { return nil } - if err := w.collectIdentifier("enumerator", alwaysNamespaced, neverLinked, noStorage, n.Name); err != nil { + if err := w.collectIdentifier(n, "enumerator", alwaysNamespaced, neverLinked, noStorage); err != nil { return err } return nil // Do not recurse. @@ -281,7 +351,7 @@ if err != nil { return fmt.Errorf("could not find storage class of function: %w: %s", err, nodeCode) } - if err := w.collectIdentifier("function", globalIfC, respectsLinkage, storage, n.Name); err != nil { + if err := w.collectIdentifier(n, "function", globalIfC, respectsLinkage, storage); err != nil { return err } case "LinkageSpecDecl": @@ -289,6 +359,9 @@ w.language = n.Language } case "NamespaceDecl": + if w.language != "C++" { + return fmt.Errorf("entering namespace while in extern %q is probably unintended: %s", w.language, nodeCode) + } if n.Name == "" { w.anonNamespace = true } else { @@ -298,7 +371,7 @@ if w.record { return nil } - if err := w.collectIdentifier("using", alwaysNamespaced, neverLinked, noStorage, n.Name); err != nil { + if err := w.collectIdentifier(n, "using", alwaysNamespaced, neverLinked, noStorage); err != nil { return err } case "TypedefDecl": @@ -309,7 +382,7 @@ // typedef struct X X; return nil } - if err := w.collectIdentifier("typedef", alwaysNamespaced, neverLinked, noStorage, n.Name); err != nil { + if err := w.collectIdentifier(n, "typedef", alwaysNamespaced, neverLinked, noStorage); err != nil { return err } case "VarDecl": @@ -320,28 +393,31 @@ if err != nil { return fmt.Errorf("could not find storage class of variable: %w: %s", err, nodeCode) } - if err := w.collectIdentifier("var", globalIfC, respectsLinkage, storage, n.Name); err != nil { + if err := w.collectIdentifier(n, "var", globalIfC, respectsLinkage, storage); err != nil { return err } return nil // Do not recurse. (Maybe should, to catch `struct ...` in variable types?) // Singletons that should be skipped. case "AccessSpecDecl", - "AlignedAttr", + "AlwaysInlineAttr", "BuiltinAttr", "BuiltinType", + "CXX11NoReturnAttr", "ConstAttr", "DependentNameType", "DeprecatedAttr", "EnumType", + "FinalAttr", "FormatAttr", "NoThrowAttr", - "ParmVarDecl", "RecordType", + "TemplateTypeParmType", "UnresolvedUsingValueDecl", "UnusedAttr", + "UsingDecl", "UsingDirectiveDecl", - "VectorType", + "VisibilityAttr", "WarnUnusedResultAttr": if len(n.Inner) != 0 { // If this ever fires, check AST to see if any of the node's children could be useful, @@ -350,6 +426,7 @@ } // Nodes that should be skipped including possible children. case + "AlignedAttr", "CXXConstructorDecl", "CXXConversionDecl", "CXXDeductionGuideDecl", @@ -358,13 +435,17 @@ "ClassTemplatePartialSpecializationDecl", "ClassTemplateSpecializationDecl", "CompoundStmt", + "DecltypeType", "FieldDecl", "FriendDecl", "NonTypeTemplateParmDecl", + "ParmVarDecl", "StaticAssertDecl", "TemplateArgument", + "TemplateTemplateParmDecl", "TemplateTypeParmDecl", - "VarTemplateDecl": + "VarTemplateDecl", + "VarTemplateSpecializationDecl": return nil // Do not recurse. // Nodes that should just be recursed into. case @@ -374,13 +455,16 @@ "ElaboratedType", "FunctionProtoType", "FunctionTemplateDecl", + "IncompleteArrayType", "IndirectFieldDecl", + "LValueReferenceType", "ParenType", "PointerType", "QualType", "TemplateSpecializationType", "TranslationUnitDecl", - "TypedefType": + "TypedefType", + "VectorType": // Just recurse. default: return fmt.Errorf("no handling for node kind %q: %s", n.Kind, nodeCode) @@ -399,7 +483,8 @@ } // collectIdentifier sends an identifier to the output. -func (w walker) collectIdentifier(tag string, namespacing namespacing, linking linking, storage storage, name string) error { +func (w walker) collectIdentifier(n *node, tag string, namespacing namespacing, linking linking, storage storage) error { + name := n.Name var fqn string if w.anonNamespace { fqn = "<anonymous>::" + name @@ -407,6 +492,15 @@ fqn = strings.Join(append(append([]string(nil), w.namespace...), name), "::") } + // With this taken out of the way, for all intents and purposes + // anything in an anonymous namespace behaves as if it were static. + // + // Helps in some cases where the static keyword + // isn't repeated in template specializations or similar. + if w.anonNamespace && w.language == "C++" { + storage = staticStorage + } + var linkage string switch linking { case neverLinked: @@ -427,7 +521,7 @@ case alwaysGlobal: identifier = name case globalIfC: - if w.language != "C" { + if w.language == "C++" { identifier = fqn } else { identifier = name @@ -446,7 +540,25 @@ return nil } w.seen[key] = declaration - fmt.Printf("%s\n", declaration) + + // Append some debug info. + // This might be used later to generate symbol renaming headers, + // but is generally useful to humans debugging this tool's output. + var suffix string + if file, line, start, end, ok := n.locationRangeWithoutBody(); ok { + suffix += fmt.Sprintf(" %s:%v (%v-%v)", file, line, start, end) + } + for _, arg := range n.functionArgs() { + argName := arg.Name + if argName == "" { + argName = "_" + } + suffix += " " + argName + } + if suffix != "" { + suffix = " //" + suffix + } + fmt.Printf("%s%s\n", declaration, suffix) return nil }