diff --git a/docs/network_traffic_annotations.md b/docs/network_traffic_annotations.md index 12514b5cd22585..c098eb4d7b4290 100644 --- a/docs/network_traffic_annotations.md +++ b/docs/network_traffic_annotations.md @@ -287,17 +287,16 @@ change list. These checks include: its unique id cannot be reused to keep the stats sound). ### Presubmit tests -To perform tests prior to submit, one can use traffic_annotation_auditor binary. -It runs over the whole repository and using a clang tool, checks if all above -items are correct. -Running the `traffic_annotation_auditor` requires having a COMPLETE compiled -build directory and can be done with the following syntax. +To perform tests prior to submit, one can use the `traffic_annotation_auditor` +binary. It runs over the whole repository and using a python script, extracts +all the annotations and then checks if all above items are correct. The latest +executable for supported platforms can be found in +`tools/traffic_annotation/bin/[platform]`. + +Running the `traffic_annotation_auditor` requires having a build directory and +can be done with the following syntax: `tools/traffic_annotation/bin/[linux64/win32]/traffic_annotation_auditor --build-path=[out/Default]` -The latest executable of `traffic_annotation_auditor` for supported platforms -can be found in `tools/traffic_annotation/bin/[platform]`. -As this test is slow, it is not a mandatory step of the presubmit checks on -clients, and one can run it manually. ### Waterfall tests Two commit queue trybots test traffic annotations on changed files using the diff --git a/tools/clang/traffic_annotation_extractor/CMakeLists.txt b/tools/clang/traffic_annotation_extractor/CMakeLists.txt deleted file mode 100644 index 32b154dae5c903..00000000000000 --- a/tools/clang/traffic_annotation_extractor/CMakeLists.txt +++ /dev/null @@ -1,28 +0,0 @@ -set(LLVM_LINK_COMPONENTS - BitReader - MCParser - Option - X86AsmParser - X86CodeGen - ) - -add_llvm_executable(traffic_annotation_extractor - traffic_annotation_extractor.cpp - ) - -target_link_libraries(traffic_annotation_extractor - clangAST - clangASTMatchers - clangAnalysis - clangBasic - clangDriver - clangEdit - clangFrontend - clangLex - clangParse - clangSema - clangSerialization - clangTooling - ) - -cr_install(TARGETS traffic_annotation_extractor RUNTIME DESTINATION bin) diff --git a/tools/clang/traffic_annotation_extractor/README.md b/tools/clang/traffic_annotation_extractor/README.md deleted file mode 100644 index cfbfbee9b519d0..00000000000000 --- a/tools/clang/traffic_annotation_extractor/README.md +++ /dev/null @@ -1,60 +0,0 @@ -# Traffic Annotation Extrator -This is a clang tool to extract network traffic annotations. The tool is run by -`tools/traffic_annotation/auditor/traffic_annotation_auditor`. Refer to it for -help on how to use. - -## Build on Linux -`tools/clang/scripts/build.py --bootstrap - --without-android --extra-tools traffic_annotation_extractor` - -## Build on Window -1. Either open a `VS2015 x64 Native Tools Command Prompt`, or open a normal - command prompt and run `depot_tools\win_toolchain\vs_files\ - $long_autocompleted_hash\win_sdk\bin\setenv.cmd /x64` -2. Run `python tools/clang/scripts/build.py --bootstrap - --without-android --extra-tools traffic_annotation_extractor` - -## Usage -Run `traffic_annotation_extractor --help` for parameters help. - -Example for direct call: - `third_party/llvm-build/Release+Asserts/bin/traffic_annotation_extractor - -p=out/Debug components/spellcheck/browser/spelling_service_client.cc` - -Example for call using run_tool.py: - `tools/clang/scripts/run_tool.py --tool=traffic_annotation_extractor - --generate-compdb -p=out/Debug components/spellcheck/browser` - -The executable extracts network traffic annotations and calls to network request - generation functions from given file paths based on build parameters in build - path, and writes them to llvm::outs. It also finds all code sites in which a - network traffic annotation tag or any of its variants are directly assigned - using a list expression constructor or assignment to its |unique_id_hash_code| - argument. - -Each annotation output will have the following format: - - Line 1: "==== NEW ANNOTATION ====" - - Line 2: File path. - - Line 3: Name of the function in which the annotation is defined. - - Line 4: Line number of the annotation. - - Line 5: Function type ("Definition", "Partial", "Completing", - "BranchedCompleting"). - - Line 6: Unique id of annotation. - - Line 7: Completing id or group id, when applicable, empty otherwise. - - Line 8-: Serialized protobuf of the annotation. (Several lines) - - Last line: "==== ANNOTATION ENDS ====" - -Each function call output will have the following format: - - Line 1: "==== NEW CALL ====" - - Line 2: File path. - - Line 3: Name of the function in which the call is made. - - Line 4: Name of the called function. - - Line 5: Does the call have an annotation? - - Line 6: "==== CALL ENDS ====" - -Each direct assignment output will have the following format: - - Line 1: "==== NEW ASSIGNMENT ====" - - Line 2: File path. - - Line 3: Name of the function in which assignment is done. - - Line 4: Line number of the assignment. - - Line 5: "==== ASSIGNMENT ENDS ====" diff --git a/tools/clang/traffic_annotation_extractor/tests/dummy_classes.h b/tools/clang/traffic_annotation_extractor/tests/dummy_classes.h deleted file mode 100644 index 773edd0cf66756..00000000000000 --- a/tools/clang/traffic_annotation_extractor/tests/dummy_classes.h +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include - -#include "net/traffic_annotation/network_traffic_annotation.h" - -// This file provides all required dummy classes for: -// tools/clang/traffic_annotation_extractor/tests/test-original.cc - -class GURL {}; - -namespace net { - -class URLRequest { - public: - class Delegate; -}; - -class URLFetcherDelegate; - -enum RequestPriority { TEST_VALUE }; - -class URLFetcher { - public: - enum RequestType { TEST_VALUE }; - - static std::unique_ptr Create( - const GURL& url, - URLFetcher::RequestType request_type, - URLFetcherDelegate* d); - - static std::unique_ptr Create( - int id, - const GURL& url, - URLFetcher::RequestType request_type, - URLFetcherDelegate* d); - - static std::unique_ptr Create( - const GURL& url, - URLFetcher::RequestType request_type, - URLFetcherDelegate* d, - NetworkTrafficAnnotationTag traffic_annotation); - - static std::unique_ptr Create( - int id, - const GURL& url, - URLFetcher::RequestType request_type, - URLFetcherDelegate* d, - NetworkTrafficAnnotationTag traffic_annotation); -}; - -class URLRequestContext { - public: - std::unique_ptr CreateRequest( - const GURL& url, - RequestPriority priority, - URLRequest::Delegate* delegate) const; - - std::unique_ptr CreateRequest( - const GURL& url, - RequestPriority priority, - URLRequest::Delegate* delegate, - NetworkTrafficAnnotationTag traffic_annotation) const; -}; - -} // namespace net \ No newline at end of file diff --git a/tools/clang/traffic_annotation_extractor/tests/test-expected.txt b/tools/clang/traffic_annotation_extractor/tests/test-expected.txt deleted file mode 100644 index c065b23d5e877e..00000000000000 --- a/tools/clang/traffic_annotation_extractor/tests/test-expected.txt +++ /dev/null @@ -1,172 +0,0 @@ -==== NEW ANNOTATION ==== -test-actual.cc -Global Namespace -14 -Definition -id1 - - - semantics { - sender: "sender1" - description: "desc1" - trigger: "trigger1" - data: "data1" - destination: GOOGLE_OWNED_SERVICE - } - policy { - cookies_allowed: NO - setting: "setting1" - chrome_policy { - SpellCheckServiceEnabled { - SpellCheckServiceEnabled: false - } - } - } - comments: "comment1" -==== ANNOTATION ENDS ==== -==== NEW ANNOTATION ==== -test-actual.cc -TestAnnotations -36 -Partial -id2 -completing_id2 - - semantics { - sender: "sender2" - description: "desc2" - trigger: "trigger2" - data: "data2" - destination: WEBSITE - } -==== ANNOTATION ENDS ==== -==== NEW ANNOTATION ==== -test-actual.cc -TestAnnotations -46 -Completing -id3 - - - policy { - cookies_allowed: YES - cookie_store: "user" - setting: "setting3" - chrome_policy { - SpellCheckServiceEnabled { - SpellCheckServiceEnabled: false - } - } - } - comments: "comment3" -==== ANNOTATION ENDS ==== -==== NEW ANNOTATION ==== -test-actual.cc -TestAnnotations -61 -BranchedCompleting -id4 -branch4 - - policy { - cookies_allowed: YES - cookie_store: "user" - setting: "setting4" - policy_exception_justification: "justification" - } -==== ANNOTATION ENDS ==== -==== NEW ANNOTATION ==== -test-actual.cc -TestURLFetcherCreate -83 -Definition -undefined - -Nothing here yet. -==== ANNOTATION ENDS ==== -==== NEW ANNOTATION ==== -test-actual.cc -TestMacroExpansion -123 -Definition -undefined - -Nothing here yet. -==== ANNOTATION ENDS ==== -==== NEW CALL ==== -test-actual.cc -TestURLFetcherCreate -73 -net::URLFetcher::Create -0 -==== CALL ENDS ==== -==== NEW CALL ==== -test-actual.cc -TestURLFetcherCreate -76 -net::URLFetcher::Create -0 -==== CALL ENDS ==== -==== NEW CALL ==== -test-actual.cc -TestURLFetcherCreate -79 -net::URLFetcher::Create -1 -==== CALL ENDS ==== -==== NEW CALL ==== -test-actual.cc -TestURLFetcherCreate -82 -net::URLFetcher::Create -1 -==== CALL ENDS ==== -==== NEW CALL ==== -test-actual.cc -TestCreateRequest -90 -net::URLRequestContext::CreateRequest -0 -==== CALL ENDS ==== -==== NEW CALL ==== -test-actual.cc -TestCreateRequest -91 -net::URLRequestContext::CreateRequest -1 -==== CALL ENDS ==== -==== NEW ASSIGNMENT ==== -test-actual.cc -TestInitList -96 -==== ASSIGNMENT ENDS ==== -==== NEW ASSIGNMENT ==== -test-actual.cc -TestInitList -97 -==== ASSIGNMENT ENDS ==== -==== NEW ASSIGNMENT ==== -test-actual.cc -TestInitList -98 -==== ASSIGNMENT ENDS ==== -==== NEW ASSIGNMENT ==== -test-actual.cc -TestInitList -99 -==== ASSIGNMENT ENDS ==== -==== NEW ASSIGNMENT ==== -test-actual.cc -TestInitList -101 -==== ASSIGNMENT ENDS ==== -==== NEW ASSIGNMENT ==== -test-actual.cc -TestAssignment -106 -==== ASSIGNMENT ENDS ==== -==== NEW ASSIGNMENT ==== -test-actual.cc -TestAssignment -109 -==== ASSIGNMENT ENDS ==== diff --git a/tools/clang/traffic_annotation_extractor/tests/test-original.cc b/tools/clang/traffic_annotation_extractor/tests/test-original.cc deleted file mode 100644 index e30262d13f2bde..00000000000000 --- a/tools/clang/traffic_annotation_extractor/tests/test-original.cc +++ /dev/null @@ -1,124 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "net/traffic_annotation/network_traffic_annotation.h" - -#include "dummy_classes.h" - -// This file provides samples for testing traffic_annotation_extractor clang -// tool. -namespace { - -net::NetworkTrafficAnnotationTag kTrafficAnnotation = - net::DefineNetworkTrafficAnnotation("id1", R"( - semantics { - sender: "sender1" - description: "desc1" - trigger: "trigger1" - data: "data1" - destination: GOOGLE_OWNED_SERVICE - } - policy { - cookies_allowed: NO - setting: "setting1" - chrome_policy { - SpellCheckServiceEnabled { - SpellCheckServiceEnabled: false - } - } - } - comments: "comment1")"); -} - -void TestAnnotations() { - net::PartialNetworkTrafficAnnotationTag partial_traffic_annotation = - net::DefinePartialNetworkTrafficAnnotation("id2", "completing_id2", R"( - semantics { - sender: "sender2" - description: "desc2" - trigger: "trigger2" - data: "data2" - destination: WEBSITE - })"); - - net::NetworkTrafficAnnotationTag completed_traffic_annotation = - net::CompleteNetworkTrafficAnnotation("id3", partial_traffic_annotation, - R"( - policy { - cookies_allowed: YES - cookie_store: "user" - setting: "setting3" - chrome_policy { - SpellCheckServiceEnabled { - SpellCheckServiceEnabled: false - } - } - } - comments: "comment3")"); - - net::NetworkTrafficAnnotationTag completed_branch_traffic_annotation = - net::BranchedCompleteNetworkTrafficAnnotation( - "id4", "branch4", partial_traffic_annotation, R"( - policy { - cookies_allowed: YES - cookie_store: "user" - setting: "setting4" - policy_exception_justification: "justification" - })"); -} - -void TestURLFetcherCreate() { - net::URLFetcherDelegate* delegate = nullptr; - net::URLFetcher::Create(GURL(), net::URLFetcher::RequestType::TEST_VALUE, - delegate); - - net::URLFetcher::Create(0, GURL(), net::URLFetcher::RequestType::TEST_VALUE, - delegate); - - net::URLFetcher::Create(GURL(), net::URLFetcher::RequestType::TEST_VALUE, - delegate, kTrafficAnnotation); - - net::URLFetcher::Create(0, GURL(), net::URLFetcher::RequestType::TEST_VALUE, - delegate, NO_TRAFFIC_ANNOTATION_YET); -} - -void TestCreateRequest() { - net::URLRequest::Delegate* delegate = nullptr; - net::URLRequestContext context; - - context.CreateRequest(GURL(), net::RequestPriority::TEST_VALUE, delegate); - context.CreateRequest(GURL(), net::RequestPriority::TEST_VALUE, delegate, - kTrafficAnnotation); -} - -void TestInitList() { - net::NetworkTrafficAnnotationTag({-1}); - net::MutableNetworkTrafficAnnotationTag({-2}); - net::PartialNetworkTrafficAnnotationTag({-1}); - net::MutablePartialNetworkTrafficAnnotationTag({-2}); - int i = 0; - net::NetworkTrafficAnnotationTag({i}); -} - -void TestAssignment() { - net::MutableNetworkTrafficAnnotationTag tag1; - tag1.unique_id_hash_code = 1; - - net::MutablePartialNetworkTrafficAnnotationTag tag2; - tag2.unique_id_hash_code = 2; - - // Test if assignment to |unique_id_hash_code| of another structure is not - // caught. - struct something_else { - int unique_id_hash_code; - } x; - - x.unique_id_hash_code = 3; -} - -void DummyFunction(net::NetworkTrafficAnnotationTag traffic_annotation) {} - -void TestMacroExpansion() { - DummyFunction(NO_TRAFFIC_ANNOTATION_YET); -} \ No newline at end of file diff --git a/tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp b/tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp deleted file mode 100644 index 5e67fa5f2a5153..00000000000000 --- a/tools/clang/traffic_annotation_extractor/traffic_annotation_extractor.cpp +++ /dev/null @@ -1,401 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -// This clang tool does the following three tasks: -// 1) Finds all instances of the following functions and extracts the location -// info and content of annotation tags: -// - net::DefineNetworkTrafficAnnotation -// - net::DefinePartialNetworkTrafficAnnotation -// - net::CompleteNetworkTrafficAnnotation -// - net::BranchedCompleteNetworkTrafficAnnotation -// 2) Extracts all calls of the following network request creation functions -// and returns their source location and availability of a -// net::[Partial]NetworkTrafficAnnotation parameter in them: -// - URLFetcher::Create -// - URLRequestContext::CreateRequest -// 3) Finds all instances of initializing any of the following classes with list -// expressions or assignment of a value to |unique_id_hash_code| of the -// mutable ones, outside traffic annotation API functions: -// - net::NetworkTrafficAnnotationTag -// - net::PartialNetworkTrafficAnnotationTag -// - net::MutableNetworkTrafficAnnotationTag -// - net::MutablePartialNetworkTrafficAnnotationTag -// All outputs are written to to llvm::outs. -// Please refer to README.md for build and usage instructions. - -#include -#include - -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/ASTMatchers/ASTMatchers.h" -#include "clang/Basic/SourceManager.h" -#include "clang/Frontend/FrontendActions.h" -#include "clang/Lex/Lexer.h" -#include "clang/Tooling/ArgumentsAdjusters.h" -#include "clang/Tooling/CommonOptionsParser.h" -#include "clang/Tooling/Refactoring.h" -#include "clang/Tooling/Tooling.h" -#include "llvm/Support/CommandLine.h" -#include "llvm/Support/TargetSelect.h" - -using namespace clang::ast_matchers; - -namespace { - -// Information about location of a line of code. -struct Location { - std::string file_path; - int line_number = -1; -}; - -// An instance of a call to either of the 4 network traffic annotation -// definition functions. -struct NetworkAnnotationInstance { - // Annotation content. These are the arguments of the call to either of the 4 - // network traffic annotation definition functions. - struct Annotation { - std::string unique_id; - std::string text; - - // |extra_id| will have |completing_id| for - // net::DefinePartialNetworkTrafficAnnotation and |group_id| for - // net::BranchedCompleteNetworkTrafficAnnotation. It will be empty in other - // cases. - std::string extra_id; - }; - - Location location; - Annotation annotation; - - // Specifying the function type. - enum FunctionType { - kDefinition, // net::DefineNetworkTrafficAnnotation - kPartial, // net::DefinePartialNetworkTrafficAnnotation - kCompleting, // net::CompleteNetworkTrafficAnnotation - kBranchedCompleting // net::BranchedCompleteNetworkTrafficAnnotation - }; - - FunctionType function_type; - - const char* GetTypeName() const { - switch (function_type) { - case kDefinition: - return "Definition"; - case kPartial: - return "Partial"; - case kCompleting: - return "Completing"; - case kBranchedCompleting: - return "BranchedCompleting"; - } - assert(false); - return ""; - } -}; - -// An instance of a call to one of the monitored function. -struct CallInstance { - // Location of the call. - Location location; - - // Whether the function is annotated. - bool has_annotation = false; - - // Name of the called function. - std::string called_function_name; -}; - -// A structure to keep detected annotation and call instances, and all code -// locations that include a direct value assignment to annotations using list -// expression constructors or mutable annotations' |unique_id_hash_code|. -struct Collector { - std::vector annotations; - std::vector calls; - std::vector assignments; -}; - -// This class implements the call back functions for AST Matchers. The matchers -// are defined in RunMatchers function. When a pattern is found there, -// the run function in this class is called back with information on the matched -// location and description of the matched pattern. -class NetworkAnnotationTagCallback : public MatchFinder::MatchCallback { - public: - explicit NetworkAnnotationTagCallback(Collector* collector) - : collector_(collector) {} - ~NetworkAnnotationTagCallback() override = default; - - // Is called on any pattern found by ASTMathers that are defined in RunMathers - // function. - virtual void run(const MatchFinder::MatchResult& result) override { - if (const clang::CallExpr* call_expr = - result.Nodes.getNodeAs("monitored_function")) { - AddFunction(call_expr, result); - } else if (const clang::CXXConstructExpr* constructor_expr = - result.Nodes.getNodeAs( - "annotation_constructor")) { - AddConstructor(constructor_expr, result); - } else if (const clang::MemberExpr* member_expr = - result.Nodes.getNodeAs( - "direct_assignment")) { - AddAssignment(member_expr, result); - } else { - AddAnnotation(result); - } - } - - void GetInstanceLocation(const MatchFinder::MatchResult& result, - const clang::Expr* expr, - Location* location) { - clang::SourceLocation source_location = expr->getBeginLoc(); - if (source_location.isMacroID()) - source_location = result.SourceManager->getExpansionLoc(source_location); - location->file_path = result.SourceManager->getFilename(source_location); - location->line_number = - result.SourceManager->getSpellingLineNumber(source_location); - - std::replace(location->file_path.begin(), location->file_path.end(), '\\', - '/'); - - // Trim leading "../"s from file path. - while (location->file_path.length() > 3 && - location->file_path.substr(0, 3) == "../") { - location->file_path = - location->file_path.substr(3, location->file_path.length() - 3); - } - } - - // Stores a function call that should be monitored. - void AddFunction(const clang::CallExpr* call_expr, - const MatchFinder::MatchResult& result) { - CallInstance instance; - - GetInstanceLocation(result, call_expr, &instance.location); - instance.called_function_name = - call_expr->getDirectCallee()->getQualifiedNameAsString(); - instance.has_annotation = - (result.Nodes.getNodeAs("annotation") != nullptr); - collector_->calls.push_back(instance); - } - - // Stores an annotation constructor called with list expression. - void AddConstructor(const clang::CXXConstructExpr* constructor_expr, - const MatchFinder::MatchResult& result) { - Location instance; - GetInstanceLocation(result, constructor_expr, &instance); - collector_->assignments.push_back(instance); - } - - // Stores a value assignment to |unique_id_hash_code| of a mutable annotaton. - void AddAssignment(const clang::MemberExpr* member_expr, - const MatchFinder::MatchResult& result) { - Location instance; - GetInstanceLocation(result, member_expr, &instance); - collector_->assignments.push_back(instance); - } - - // Stores an annotation. - void AddAnnotation(const MatchFinder::MatchResult& result) { - NetworkAnnotationInstance instance; - - const clang::StringLiteral* unique_id = - result.Nodes.getNodeAs("unique_id"); - const clang::StringLiteral* annotation_text = - result.Nodes.getNodeAs("annotation_text"); - const clang::StringLiteral* group_id = - result.Nodes.getNodeAs("group_id"); - const clang::StringLiteral* completing_id = - result.Nodes.getNodeAs("completing_id"); - - const clang::CallExpr* call_expr = nullptr; - if ((call_expr = - result.Nodes.getNodeAs("definition_function"))) { - instance.function_type = NetworkAnnotationInstance::kDefinition; - } else if ((call_expr = result.Nodes.getNodeAs( - "partial_function"))) { - instance.function_type = NetworkAnnotationInstance::kPartial; - assert(completing_id); - instance.annotation.extra_id = completing_id->getString(); - } else if ((call_expr = result.Nodes.getNodeAs( - "completing_function"))) { - instance.function_type = NetworkAnnotationInstance::kCompleting; - } else if ((call_expr = result.Nodes.getNodeAs( - "branched_completing_function"))) { - instance.function_type = NetworkAnnotationInstance::kBranchedCompleting; - assert(group_id); - instance.annotation.extra_id = group_id->getString(); - } else { - assert(false); - } - - assert(unique_id && annotation_text); - instance.annotation.unique_id = unique_id->getString(); - instance.annotation.text = annotation_text->getString(); - - GetInstanceLocation(result, call_expr, &instance.location); - - collector_->annotations.push_back(instance); - } - - private: - Collector* collector_; -}; - -// Sets up an ASTMatcher and runs clang tool to populate collector. Returns the -// result of running the clang tool. -int RunMatchers(clang::tooling::ClangTool* clang_tool, Collector* collector) { - NetworkAnnotationTagCallback callback(collector); - MatchFinder match_finder; - - // Set up patterns to find network traffic annotation definition functions, - // their arguments, and their ancestor function (when possible). - auto bind_function_context_if_present = - anyOf(hasAncestor(functionDecl().bind("function_context")), - unless(hasAncestor(functionDecl()))); - auto has_annotation_parameter = anyOf( - hasAnyParameter(hasType( - recordDecl(anyOf(hasName("net::NetworkTrafficAnnotationTag"), - hasName("net::PartialNetworkTrafficAnnotationTag"))) - .bind("annotation"))), - unless(hasAnyParameter(hasType(recordDecl( - anyOf(hasName("net::NetworkTrafficAnnotationTag"), - hasName("net::PartialNetworkTrafficAnnotationTag"))))))); - match_finder.addMatcher( - callExpr(hasDeclaration(functionDecl( - anyOf(hasName("DefineNetworkTrafficAnnotation"), - hasName("net::DefineNetworkTrafficAnnotation")))), - hasArgument(0, stringLiteral().bind("unique_id")), - hasArgument(1, stringLiteral().bind("annotation_text")), - bind_function_context_if_present) - .bind("definition_function"), - &callback); - match_finder.addMatcher( - callExpr(hasDeclaration(functionDecl(anyOf( - hasName("DefinePartialNetworkTrafficAnnotation"), - hasName("net::DefinePartialNetworkTrafficAnnotation")))), - hasArgument(0, stringLiteral().bind("unique_id")), - hasArgument(1, stringLiteral().bind("completing_id")), - hasArgument(2, stringLiteral().bind("annotation_text")), - bind_function_context_if_present) - .bind("partial_function"), - &callback); - match_finder.addMatcher( - callExpr(hasDeclaration(functionDecl( - anyOf(hasName("CompleteNetworkTrafficAnnotation"), - hasName("net::CompleteNetworkTrafficAnnotation")))), - hasArgument(0, stringLiteral().bind("unique_id")), - hasArgument(2, stringLiteral().bind("annotation_text")), - bind_function_context_if_present) - .bind("completing_function"), - &callback); - match_finder.addMatcher( - callExpr(hasDeclaration(functionDecl(anyOf( - hasName("BranchedCompleteNetworkTrafficAnnotation"), - hasName("net::BranchedCompleteNetworkTrafficAnnotation")))), - hasArgument(0, stringLiteral().bind("unique_id")), - hasArgument(1, stringLiteral().bind("group_id")), - hasArgument(3, stringLiteral().bind("annotation_text")), - bind_function_context_if_present) - .bind("branched_completing_function"), - &callback); - - // Setup patterns to find functions that should be monitored. - match_finder.addMatcher( - callExpr(hasDeclaration(functionDecl( - anyOf(hasName("URLFetcher::Create"), - hasName("URLRequestContext::CreateRequest")), - has_annotation_parameter)), - bind_function_context_if_present) - .bind("monitored_function"), - &callback); - - // Setup patterns to find constructors of different network traffic annotation - // tags that are initialized by list expressions. - match_finder.addMatcher( - cxxConstructExpr( - hasDeclaration(functionDecl( - anyOf(hasName("net::NetworkTrafficAnnotationTag::" - "NetworkTrafficAnnotationTag"), - hasName("net::PartialNetworkTrafficAnnotationTag::" - "PartialNetworkTrafficAnnotationTag"), - hasName("net::MutableNetworkTrafficAnnotationTag::" - "MutableNetworkTrafficAnnotationTag"), - hasName("net::MutablePartialNetworkTrafficAnnotationTag::" - "MutablePartialNetworkTrafficAnnotationTag")))), - hasDescendant(initListExpr()), bind_function_context_if_present) - .bind("annotation_constructor"), - &callback); - - // Setup pattern to find direct assignment of value to |unique_id_hash_code| - // of net::MutableNetworkTrafficAnnotationTag or - // net::MutablePartialNetworkTrafficAnnotationTag. - match_finder.addMatcher( - memberExpr( - member(hasName("unique_id_hash_code")), - hasObjectExpression(hasType(cxxRecordDecl(anyOf( - hasName("net::MutableNetworkTrafficAnnotationTag"), - hasName("net::MutablePartialNetworkTrafficAnnotationTag"))))), - hasParent(binaryOperator(hasOperatorName("="))), - bind_function_context_if_present) - .bind("direct_assignment"), - &callback); - - std::unique_ptr frontend_factory = - clang::tooling::newFrontendActionFactory(&match_finder); - return clang_tool->run(frontend_factory.get()); -} - -} // namespace - -static llvm::cl::OptionCategory ToolCategory( - "traffic_annotation_extractor: Extract traffic annotation texts"); -static llvm::cl::extrahelp CommonHelp( - clang::tooling::CommonOptionsParser::HelpMessage); - -int main(int argc, const char* argv[]) { - clang::tooling::CommonOptionsParser options(argc, argv, ToolCategory); - clang::tooling::ClangTool tool(options.getCompilations(), - options.getSourcePathList()); - tool.appendArgumentsAdjuster(clang::tooling::getStripPluginsAdjuster()); - Collector collector; - - llvm::InitializeNativeTarget(); - llvm::InitializeNativeTargetAsmParser(); - int result = RunMatchers(&tool, &collector); - - if (result != 0) - return result; - - // For each call to any of the functions that define a network traffic - // annotation, write annotation text and relevant meta data into llvm::outs(). - for (const NetworkAnnotationInstance& instance : collector.annotations) { - llvm::outs() << "==== NEW ANNOTATION ====\n"; - llvm::outs() << instance.location.file_path << "\n"; - llvm::outs() << instance.location.line_number << "\n"; - llvm::outs() << instance.GetTypeName() << "\n"; - llvm::outs() << instance.annotation.unique_id << "\n"; - llvm::outs() << instance.annotation.extra_id << "\n"; - llvm::outs() << instance.annotation.text << "\n"; - llvm::outs() << "==== ANNOTATION ENDS ====\n"; - } - - // For each call, write annotation text and relevant meta data. - for (const CallInstance& instance : collector.calls) { - llvm::outs() << "==== NEW CALL ====\n"; - llvm::outs() << instance.location.file_path << "\n"; - llvm::outs() << instance.location.line_number << "\n"; - llvm::outs() << instance.called_function_name << "\n"; - llvm::outs() << instance.has_annotation << "\n"; - llvm::outs() << "==== CALL ENDS ====\n"; - } - - // For each assignment, write relevant meta data. - for (const Location& instance : collector.assignments) { - llvm::outs() << "==== NEW ASSIGNMENT ====\n"; - llvm::outs() << instance.file_path << "\n"; - llvm::outs() << instance.line_number << "\n"; - llvm::outs() << "==== ASSIGNMENT ENDS ====\n"; - } - - return 0; -} diff --git a/tools/traffic_annotation/README.md b/tools/traffic_annotation/README.md index 2b8c5224d97168..19832842c3595b 100644 --- a/tools/traffic_annotation/README.md +++ b/tools/traffic_annotation/README.md @@ -7,19 +7,19 @@ requires annotation, is annotated, and annotations are sound and complete. # Traffic Annotation Auditor This is the main executable for all the tests. It runs Traffic Annotation -Extractor clang tool to check the repository, extract annotations, and perform -required tests and maintenance. See more details in +Extractor python script to check the repository, extract annotations, and +perform required tests and maintenance. See more details in `tools/traffic_annotation/auditor/README.md`. # Traffic Annotation Extractor -Traffic Annotation Auditor uses this clang tool (located in -`tools\clang\traffic_annotation_extractor`) to parse the code and extract +Traffic Annotation Auditor uses this python script (located in +`tools/traffic_annotation/scripts/extractor.py`) to parse the code and extract required data for testing and maintenance. # Building the Checkers -We do not want every developer to have to build clang tool and auditor, and so -we store pre-built binaries in a Google Cloud Storage bucket and retrieve them -via gclient hooks. The binaries are in `tools/traffic_annotation/bin/[platform]` +We do not want every developer to have to build the auditor, and so we store +pre-built binaries in a Google Cloud Storage bucket and retrieve them via +gclient hooks. The binaries are in `tools/traffic_annotation/bin/[platform]` folder. To roll new versions of the binaries, please see `tools/traffic_annotation/bin/README.md`. diff --git a/tools/traffic_annotation/auditor/README.md b/tools/traffic_annotation/auditor/README.md index 700f09fb9d0ea1..0e9a82d810e5ee 100644 --- a/tools/traffic_annotation/auditor/README.md +++ b/tools/traffic_annotation/auditor/README.md @@ -1,5 +1,5 @@ # Network Traffic Annotation Auditor -This binary runs the clang tool for extraction of Network Traffic Annotations +This binary runs extractor.py for extraction of Network Traffic Annotations from chromium source code, collects and summarizes its outputs, and performs tests and maintenance. Please see `docs/network_traffic_annotations.md` for an introduction to network @@ -16,7 +16,7 @@ Run `traffic_annotation_auditor --help` for options. Example: `traffic_annotation_auditor --build-path=out/Debug` -The binaries of this file and the clang tool are checked out into +The binary for this file is checked out into `tools/traffic_annotation/bin/[platform]`. This is only done for Linux and Windows platforms now and will be extended to other platforms later. diff --git a/tools/traffic_annotation/auditor/instance.h b/tools/traffic_annotation/auditor/instance.h index 74e003ce864cc9..5f674c2f9a30f0 100644 --- a/tools/traffic_annotation/auditor/instance.h +++ b/tools/traffic_annotation/auditor/instance.h @@ -37,7 +37,7 @@ class AnnotationInstance : public InstanceBase { AnnotationInstance(const AnnotationInstance& other); // Deserializes an instance from serialized lines of the text provided by the - // clang tool. + // extractor. // |serialized_lines| are read from |start_line| to |end_line| and should // contain the following lines: // 1- File path. @@ -142,7 +142,7 @@ class CallInstance : public InstanceBase { CallInstance(const CallInstance& other); // Deserializes an instance from serialized lines of text provided by the - // clang tool. + // extractor. // |serialized_lines| are read from |start_line| to |end_line| and should // contain the following lines: // 1- File path. @@ -172,7 +172,7 @@ class AssignmentInstance : public InstanceBase { AssignmentInstance(const AssignmentInstance& other); // Deserializes an instance from serialized lines of text provided by the - // clang tool. + // extractor. // |serialized_lines| are read from |start_line| to |end_line| and should // contain the following lines: // 1- File path. diff --git a/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc b/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc index 347ddb6786bf04..9d8aae979f5bba 100644 --- a/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc +++ b/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc @@ -60,28 +60,6 @@ const base::FilePath kSafeListPath = .Append(FILE_PATH_LITERAL("auditor")) .Append(FILE_PATH_LITERAL("safe_list.txt")); -const base::FilePath kClangToolSwitchesPath = - base::FilePath(FILE_PATH_LITERAL("tools")) - .Append(FILE_PATH_LITERAL("traffic_annotation")) - .Append(FILE_PATH_LITERAL("auditor")) - .Append(FILE_PATH_LITERAL("traffic_annotation_extractor_switches.txt")); - -// The folder that includes the latest Clang built-in library. Inside this -// folder, there should be another folder with version number, like -// '.../lib/clang/6.0.0', which would be passed to the clang tool. -const base::FilePath kClangLibraryPath = - base::FilePath(FILE_PATH_LITERAL("third_party")) - .Append(FILE_PATH_LITERAL("llvm-build")) - .Append(FILE_PATH_LITERAL("Release+Asserts")) - .Append(FILE_PATH_LITERAL("lib")) - .Append(FILE_PATH_LITERAL("clang")); - -const base::FilePath kRunToolScript = - base::FilePath(FILE_PATH_LITERAL("tools")) - .Append(FILE_PATH_LITERAL("clang")) - .Append(FILE_PATH_LITERAL("scripts")) - .Append(FILE_PATH_LITERAL("run_tool.py")); - const base::FilePath kExtractorScript = base::FilePath(FILE_PATH_LITERAL("tools")) .Append(FILE_PATH_LITERAL("traffic_annotation")) @@ -137,17 +115,14 @@ std::string MakeRelativePath(const base::FilePath& base_directory, TrafficAnnotationAuditor::TrafficAnnotationAuditor( const base::FilePath& source_path, const base::FilePath& build_path, - const base::FilePath& clang_tool_path, const std::vector& path_filters) : source_path_(source_path), build_path_(build_path), - clang_tool_path_(clang_tool_path), path_filters_(path_filters), exporter_(source_path), safe_list_loaded_(false) { DCHECK(!source_path.empty()); DCHECK(!build_path.empty()); - DCHECK(!clang_tool_path.empty()); // Get absolute source path. base::FilePath original_path; @@ -157,16 +132,6 @@ TrafficAnnotationAuditor::TrafficAnnotationAuditor( base::SetCurrentDirectory(original_path); absolute_source_path_ = absolute_source_path_.NormalizePathSeparatorsTo('/'); DCHECK(absolute_source_path_.IsAbsolute()); - - base::FilePath switches_file = - base::MakeAbsoluteFilePath(source_path_.Append(kClangToolSwitchesPath)); - std::string file_content; - if (base::ReadFileToString(switches_file, &file_content)) { - clang_tool_switches_ = base::SplitString( - file_content, "\n", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY); - } else { - LOG(ERROR) << "Could not read " << kClangToolSwitchesPath; - } } TrafficAnnotationAuditor::~TrafficAnnotationAuditor() = default; @@ -178,32 +143,22 @@ int TrafficAnnotationAuditor::ComputeHashValue(const std::string& unique_id) { : -1; } -base::FilePath TrafficAnnotationAuditor::GetClangLibraryPath() { - return base::FileEnumerator(source_path_.Append(kClangLibraryPath), false, - base::FileEnumerator::DIRECTORIES) - .Next(); -} - bool TrafficAnnotationAuditor::RunExtractor( - ExtractorBackend backend, bool filter_files_based_on_heuristics, bool use_compile_commands, const base::FilePath& errors_file, int* exit_code) { - DCHECK(backend == ExtractorBackend::CLANG_TOOL || - backend == ExtractorBackend::PYTHON_SCRIPT); - if (!safe_list_loaded_ && !LoadSafeList()) return false; // Get list of files/folders to process. std::vector file_paths; - GenerateFilesListForClangTool(backend, filter_files_based_on_heuristics, + GenerateFilesListForExtractor(filter_files_based_on_heuristics, use_compile_commands, &file_paths); if (file_paths.empty()) return true; - // Create a file to pass options to the clang tool running script. + // Create a file to pass options to the extractor script. base::FilePath options_filepath; if (!base::CreateTemporaryFile(&options_filepath)) { LOG(ERROR) << "Could not create temporary options file."; @@ -215,23 +170,17 @@ bool TrafficAnnotationAuditor::RunExtractor( return false; } - // Write some options to the file, which depends on the backend used. - if (backend == ExtractorBackend::CLANG_TOOL) - WriteClangToolOptions(options_file, use_compile_commands); - else if (backend == ExtractorBackend::PYTHON_SCRIPT) - WritePythonScriptOptions(options_file); + // Write some options to the file for the python extractor. + WritePythonScriptOptions(options_file); - // Write the file paths regardless of backend. + // Write the file paths. for (const std::string& file_path : file_paths) fprintf(options_file, "%s ", file_path.c_str()); base::CloseFile(options_file); - const base::FilePath& script_path = - (backend == ExtractorBackend::CLANG_TOOL ? kRunToolScript - : kExtractorScript); base::CommandLine cmdline( - base::MakeAbsoluteFilePath(source_path_.Append(script_path))); + base::MakeAbsoluteFilePath(source_path_.Append(kExtractorScript))); #if defined(OS_WIN) cmdline.PrependWrapper(L"python"); #endif @@ -250,51 +199,35 @@ bool TrafficAnnotationAuditor::RunExtractor( // not perform the task. if (extractor_raw_output_.empty()) { result = false; - } else if (backend == ExtractorBackend::CLANG_TOOL && !result) { - // If clang tool had errors but also returned results, the errors can be - // ignored as we do not separate platform specific files here and processing - // them fails. This is a post-build test and if there exists any actual - // compile error, it should be noted when the code is built. - printf("WARNING: Ignoring clang tool's returned errors.\n"); - result = true; } if (!result) { - if (backend == ExtractorBackend::CLANG_TOOL && use_compile_commands && - !extractor_raw_output_.empty()) { - printf( - "\nWARNING: Ignoring clang tool error as it is called using " - "compile_commands.json which will result in processing some " - "library files that clang cannot process.\n"); - result = true; - } else { - std::string tool_errors; - std::string options_file_text; + std::string tool_errors; + std::string options_file_text; - base::GetAppOutputAndError(cmdline, &tool_errors); + base::GetAppOutputAndError(cmdline, &tool_errors); - if (!base::ReadFileToString(options_filepath, &options_file_text)) - options_file_text = "Could not read options file."; + if (!base::ReadFileToString(options_filepath, &options_file_text)) + options_file_text = "Could not read options file."; - std::string error_message = base::StringPrintf( - "Calling clang tool returned false from %s\nCommandline: %s\n\n" - "Returned output: %s\n\nPartial options file: %s\n", - source_path_.MaybeAsASCII().c_str(), + std::string error_message = base::StringPrintf( + "Calling extractor returned false from %s\nCommandline: %s\n\n" + "Returned output: %s\n\nPartial options file: %s\n", + source_path_.MaybeAsASCII().c_str(), #if defined(OS_WIN) - base::UTF16ToASCII(cmdline.GetCommandLineString()).c_str(), + base::UTF16ToASCII(cmdline.GetCommandLineString()).c_str(), #else - cmdline.GetCommandLineString().c_str(), + cmdline.GetCommandLineString().c_str(), #endif - tool_errors.c_str(), options_file_text.substr(0, 1024).c_str()); + tool_errors.c_str(), options_file_text.substr(0, 1024).c_str()); - if (errors_file.empty()) { - LOG(ERROR) << error_message; - } else { - if (base::WriteFile(errors_file, error_message.c_str(), - error_message.length()) == -1) { - LOG(ERROR) << "Writing error message to file failed:\n" - << error_message; - } + if (errors_file.empty()) { + LOG(ERROR) << error_message; + } else { + if (base::WriteFile(errors_file, error_message.c_str(), + error_message.length()) == -1) { + LOG(ERROR) << "Writing error message to file failed:\n" + << error_message; } } } @@ -305,58 +238,16 @@ bool TrafficAnnotationAuditor::RunExtractor( return result; } -void TrafficAnnotationAuditor::WriteClangToolOptions( - FILE* options_file, - bool use_compile_commands) { - // As the checked out clang tool may be in a directory different from the - // default one (third_party/llvm-build/Release+Asserts/bin), its path and - // clang's library folder should be passed to the run_tool.py script. - fprintf( - options_file, - "--generate-compdb --tool=traffic_annotation_extractor -p=%s " - "--tool-path=%s " - "--tool-arg=--extra-arg=-resource-dir=%s ", - build_path_.MaybeAsASCII().c_str(), - base::MakeAbsoluteFilePath(clang_tool_path_).MaybeAsASCII().c_str(), - base::MakeAbsoluteFilePath(GetClangLibraryPath()).MaybeAsASCII().c_str()); - - for (const std::string& item : clang_tool_switches_) - fprintf(options_file, "--tool-arg=--extra-arg=%s ", item.c_str()); - - if (use_compile_commands) - fprintf(options_file, "--all "); -} - void TrafficAnnotationAuditor::WritePythonScriptOptions(FILE* options_file) { fprintf(options_file, "--generate-compdb --build-path=%s ", build_path_.MaybeAsASCII().c_str()); } -void TrafficAnnotationAuditor::GenerateFilesListForClangTool( - ExtractorBackend backend, +void TrafficAnnotationAuditor::GenerateFilesListForExtractor( bool filter_files_based_on_heuristics, bool use_compile_commands, std::vector* file_paths) { TrafficAnnotationFileFilter filter; - - // When using the Clang tool backend and |use_compile_commands| is requested - // or |filter_files_based_on_heuristics| is false, we pass all given file - // paths to the running script and the files in the safe list will be later - // removed from the results. The Python tool requires a good list of file - // paths and cannot implement the same logic. - if (backend == ExtractorBackend::CLANG_TOOL && - (!filter_files_based_on_heuristics || use_compile_commands)) { - if (path_filters_.empty()) { - // If no path filter is specified, return current location. The clang tool - // will be run from the repository 'src' folder and hence this will point - // to repository root. - file_paths->push_back("./"); - } else { - *file_paths = path_filters_; - } - return; - } - // If no path filter is provided, get all relevant files, except the safe // listed ones. if (path_filters_.empty()) { @@ -425,7 +316,7 @@ bool TrafficAnnotationAuditor::IsSafeListed( return false; } -bool TrafficAnnotationAuditor::ParseClangToolRawOutput() { +bool TrafficAnnotationAuditor::ParseExtractorRawOutput() { if (!safe_list_loaded_ && !LoadSafeList()) return false; // Remove possible carriage return characters before splitting lines. @@ -434,7 +325,7 @@ bool TrafficAnnotationAuditor::ParseClangToolRawOutput() { std::vector lines = base::SplitString( temp_string, "\n", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL); for (unsigned int current = 0; current < lines.size(); current++) { - // All blocks reported by clang tool start with '====', so we can ignore + // All blocks reported by extractor start with '====', so we can ignore // all lines that do not start with a '='. if (lines[current].empty() || lines[current][0] != '=') continue; diff --git a/tools/traffic_annotation/auditor/traffic_annotation_auditor.h b/tools/traffic_annotation/auditor/traffic_annotation_auditor.h index 0bb2fac426069b..f5ae21957a00c4 100644 --- a/tools/traffic_annotation/auditor/traffic_annotation_auditor.h +++ b/tools/traffic_annotation/auditor/traffic_annotation_auditor.h @@ -14,12 +14,6 @@ #include "tools/traffic_annotation/auditor/traffic_annotation_exporter.h" #include "tools/traffic_annotation/traffic_annotation.pb.h" -enum class ExtractorBackend { - CLANG_TOOL, - PYTHON_SCRIPT, - INVALID, -}; - // Holds an item of safe list rules for auditor. struct AuditorException { enum class ExceptionType { @@ -56,34 +50,30 @@ class TrafficAnnotationAuditor { // Creates an auditor object, storing the following paths: // |source_path|: Path to the src directory. // |build_path|: Path to a compiled build directory. - // |clang_tool_path|: Path to the 'traffic_annotation_extractor' clang tool. // |path_filters|: Filters to limit where we're scanning the source. TrafficAnnotationAuditor(const base::FilePath& source_path, const base::FilePath& build_path, - const base::FilePath& clang_tool_path, const std::vector& path_filters); ~TrafficAnnotationAuditor(); - // Runs traffic_annotation_extractor clang tool (or extractor.py script) and - // puts its output in |extractor_raw_output_|. If + // Runs extractor.py and puts its output in |extractor_raw_output_|. If // |filter_files_based_on_heuristics| flag is set, the list of files will be // received from repository and heuristically filtered to only process the // relevant files. If |use_compile_commands| flag is set, the list of files is // extracted from compile_commands.json instead of git and will not be - // filtered. If clang tool returns errors, the tool is run again to record - // errors. Errors are written to |errors_file| if it is not empty, otherwise + // filtered. If the extractor returns errors, the tool is run again to record + // errors. Errors are written to |errors_file| if it is not empty, otherwise // LOG(ERROR). - bool RunExtractor(ExtractorBackend backend, - bool filter_files_based_on_heuristics, + bool RunExtractor(bool filter_files_based_on_heuristics, bool use_compile_commands, const base::FilePath& errors_file, int* exit_code); - // Parses the output of clang tool (|extractor_raw_output_|) and populates + // Parses the output of extractor.py (|extractor_raw_output_|) and populates // |extracted_annotations_|, |extracted_calls_|, and |errors_|. // Errors include not finding the file, incorrect content, or missing or not // provided annotations. - bool ParseClangToolRawOutput(); + bool ParseExtractorRawOutput(); // Computes the hash value of a traffic annotation unique id. static int ComputeHashValue(const std::string& unique_id); @@ -163,21 +153,15 @@ class TrafficAnnotationAuditor { gn_file_for_test_ = file_path; } - // Returns the path to clang internal libraries. - base::FilePath GetClangLibraryPath(); - void ClearPathFilters() { path_filters_.clear(); } private: const base::FilePath source_path_; const base::FilePath build_path_; - const base::FilePath clang_tool_path_; std::vector path_filters_; base::FilePath absolute_source_path_; - std::vector clang_tool_switches_; - TrafficAnnotationExporter exporter_; std::string extractor_raw_output_; @@ -199,16 +183,13 @@ class TrafficAnnotationAuditor { // 3- Path matches an item in |path_filters_|. void AddMissingAnnotations(); - // Generates files list to Run clang tool on. Please refer to RunExtractor + // Generates files list to run extractor on. Please refer to RunExtractor // function's comment. - void GenerateFilesListForClangTool( - ExtractorBackend backend, - bool filter_files_based_on_heuristics, - bool use_compile_commands, - std::vector* file_paths); + void GenerateFilesListForExtractor(bool filter_files_based_on_heuristics, + bool use_compile_commands, + std::vector* file_paths); // Write flags to the options file, for RunExtractor. - void WriteClangToolOptions(FILE* options_file, bool use_compile_commands); void WritePythonScriptOptions(FILE* options_file); base::FilePath gn_file_for_test_; diff --git a/tools/traffic_annotation/auditor/traffic_annotation_auditor_ui.cc b/tools/traffic_annotation/auditor/traffic_annotation_auditor_ui.cc index d5f411958f6df5..d369798180a702 100644 --- a/tools/traffic_annotation/auditor/traffic_annotation_auditor_ui.cc +++ b/tools/traffic_annotation/auditor/traffic_annotation_auditor_ui.cc @@ -39,22 +39,19 @@ specified, only those directories of the source will be analyzed. --source-path Optional path to the src directory. If not provided and build-path is available, assumed to be 'build-path/../..', otherwise current directory. - --tool-path Optional path to traffic_annotation_extractor clang tool. - If not specified, it's assumed to be in the same path as - auditor's executable. --extractor-output Optional path to the temporary file that extracted annotations will be stored into. --extractor-input Optional path to the file that temporary extracted annotations are already stored in. If this is provided, - clang tool is not run and this is used as input. + the extractor is not run and this is used as input. --no-filtering Optional flag asking the tool to run on the whole - repository without text filtering files. Using this flag - may increase processing time x40. + repository without text filtering files. --all-files Optional flag asking to use compile_commands.json instead of git to get the list of files that will be checked. This flag is useful when using build flags that change files, like jumbo. This flag slows down the execution as - it checks every compiled file. + it checks every compiled file. When enabled, path_filters + are ignored. --test-only Optional flag to request just running tests and not updating any file. If not specified, 'tools/traffic_annotation/summary/annotations.xml' might @@ -70,10 +67,6 @@ specified, only those directories of the source will be analyzed. --error-resilient Optional flag, stating not to return error in exit code if auditor fails to perform the tests. This flag can be used for trybots to avoid spamming when tests cannot run. - --extractor-backend=[clang_tool,python_script] - Optional flag specifying which backend to use for - extracting annotation definitions from source code (Clang - Tool or extractor.py). Defaults to "python_script". path_filters Optional paths to filter which files the tool is run on. It can also include deleted files names when auditor is run on a partial repository. These are ignored if all of @@ -142,14 +135,6 @@ std::string UpdateTextForTSV(std::string text) { return text; } -ExtractorBackend GetExtractorBackend(const std::string& backend_switch) { - if (backend_switch == "clang_tool") - return ExtractorBackend::CLANG_TOOL; - if (backend_switch.empty() || backend_switch == "python_script") - return ExtractorBackend::PYTHON_SCRIPT; - return ExtractorBackend::INVALID; -} - // TODO(rhalavati): Update this function to extract the policy name and value // directly from the ChromeSettingsProto object (gen/components/policy/proto/ // chrome_settings.proto). Since ChromeSettingsProto has over 300+ @@ -324,7 +309,6 @@ int main(int argc, char* argv[]) { base::FilePath build_path = command_line.GetSwitchValuePath("build-path"); base::FilePath source_path = command_line.GetSwitchValuePath("source-path"); - base::FilePath tool_path = command_line.GetSwitchValuePath("tool-path"); base::FilePath extractor_output = command_line.GetSwitchValuePath("extractor-output"); base::FilePath extractor_input = @@ -366,11 +350,6 @@ int main(int argc, char* argv[]) { path_filters = command_line.GetArgs(); #endif - // If tool path is not specified, assume it is in the same path as this - // executable. - if (tool_path.empty()) - tool_path = command_line.GetProgram().DirName(); - // Get build directory, if it is empty issue an error. if (build_path.empty()) { LOG(ERROR) @@ -386,29 +365,18 @@ int main(int argc, char* argv[]) { .Append(base::FilePath::kParentDirectory); } - TrafficAnnotationAuditor auditor(source_path, build_path, tool_path, - path_filters); + TrafficAnnotationAuditor auditor(source_path, build_path, path_filters); // Extract annotations. if (extractor_input.empty()) { - std::string backend_switch = - command_line.GetSwitchValueASCII("extractor-backend"); - ExtractorBackend backend = GetExtractorBackend(backend_switch); - if (backend == ExtractorBackend::INVALID) { - LOG(ERROR) << "Unrecognized extractor backend '" << backend_switch << "'"; - return error_value; - } - - // If we're using the Python backend, it's fast enough that we can ignore - // any path filters when we say we want to audit everything. - if (backend == ExtractorBackend::PYTHON_SCRIPT && - (!filter_files || all_files)) { + // We ignore any path filters when all files are requested. + if (!filter_files || all_files) { LOG(WARNING) << "The path_filters input is being ignored."; auditor.ClearPathFilters(); } int extractor_exit_code = EXIT_SUCCESS; - if (!auditor.RunExtractor(backend, filter_files, all_files, errors_file, + if (!auditor.RunExtractor(filter_files, all_files, errors_file, &extractor_exit_code)) { LOG(ERROR) << "Failed to run extractor.py. (exit code " << extractor_exit_code << ")"; @@ -440,7 +408,7 @@ int main(int argc, char* argv[]) { } // Process extractor output. - if (!auditor.ParseClangToolRawOutput()) + if (!auditor.ParseExtractorRawOutput()) return error_value; // Perform checks. diff --git a/tools/traffic_annotation/auditor/traffic_annotation_auditor_unittest.cc b/tools/traffic_annotation/auditor/traffic_annotation_auditor_unittest.cc index add0cf96afaf6d..502848f5033209 100644 --- a/tools/traffic_annotation/auditor/traffic_annotation_auditor_unittest.cc +++ b/tools/traffic_annotation/auditor/traffic_annotation_auditor_unittest.cc @@ -43,10 +43,6 @@ const base::FilePath kTestsFolder = .Append(FILE_PATH_LITERAL("auditor")) .Append(FILE_PATH_LITERAL("tests")); -const base::FilePath kClangToolPath = - base::FilePath(FILE_PATH_LITERAL("tools")) - .Append(FILE_PATH_LITERAL("traffic_annotation/bin")); - const std::set kDummyDeprecatedIDs = {100, 101, 102}; } // namespace @@ -59,19 +55,6 @@ class TrafficAnnotationAuditorTest : public ::testing::Test { } tests_folder_ = source_path_.Append(kTestsFolder); - -#if defined(OS_WIN) - base::FilePath platform_name(FILE_PATH_LITERAL("win32")); -#elif defined(OS_LINUX) - base::FilePath platform_name(FILE_PATH_LITERAL("linux64")); -#elif defined(OS_MACOSX) - base::FilePath platform_name(FILE_PATH_LITERAL("mac")); -#else - NOTREACHED() << "Unexpected platform."; -#endif - - base::FilePath clang_tool_path = - source_path_.Append(kClangToolPath).Append(platform_name); std::vector path_filters; // As build path is not available and not used in tests, the default (empty) @@ -80,7 +63,7 @@ class TrafficAnnotationAuditorTest : public ::testing::Test { source_path_, source_path_.Append(FILE_PATH_LITERAL("out")) .Append(FILE_PATH_LITERAL("Default")), - clang_tool_path, path_filters); + path_filters); id_checker_ = std::make_unique( TrafficAnnotationAuditor::GetReservedIDsSet(), kDummyDeprecatedIDs); @@ -94,7 +77,7 @@ class TrafficAnnotationAuditorTest : public ::testing::Test { protected: // Deserializes an annotation or a call instance from a sample file similar to - // clang tool outputs. + // extractor outputs. AuditorResult::Type Deserialize(const std::string& file_name, InstanceBase* instance); diff --git a/tools/traffic_annotation/bin/README.md b/tools/traffic_annotation/bin/README.md index 27b57fd49a9978..831ad3ffa0bc95 100644 --- a/tools/traffic_annotation/bin/README.md +++ b/tools/traffic_annotation/bin/README.md @@ -1,6 +1,7 @@ ## Building the annotation checker. -We do not want every developer to have to build clang, and so we store pre-built -binaries in a Google Cloud Storage bucket and retrieve them via gclient hooks. +We do not want every developer to have to build the auditor, and so we store the +pre-built binary in a Google Cloud Storage bucket and retrieve it via gclient +hooks. To roll new versions of the binaries, you need to have write access to the chromium-tools-traffic_annotation bucket. If you don't, contact the OWNERS list @@ -9,10 +10,6 @@ in this folder, otherwise run: # On Linux: ```bash git new-branch roll_traffic_annotation_tools -python tools/clang/scripts/build.py --bootstrap \ - --without-android --without-fuchsia --extra-tools traffic_annotation_extractor -cp third_party/llvm-build/Release+Asserts/bin/traffic_annotation_extractor \ - tools/traffic_annotation/bin/linux64/ # These GN flags produce an optimized, stripped binary that has no dependency # on glib. @@ -22,14 +19,12 @@ ninja -C out/Default traffic_annotation_auditor cp -p out/Default/traffic_annotation_auditor \ tools/traffic_annotation/bin/linux64 -strip tools/traffic_annotation/bin/linux64/traffic_annotation_{auditor,extractor} +strip tools/traffic_annotation/bin/linux64/traffic_annotation_auditor third_party/depot_tools/upload_to_google_storage.py \ -b chromium-tools-traffic_annotation \ - tools/traffic_annotation/bin/linux64/traffic_annotation_{auditor,extractor} -sed -i '/^CLANG_REVISION =/d' tools/traffic_annotation/bin/README.md + tools/traffic_annotation/bin/linux64/traffic_annotation_auditor sed -i '/^LASTCHANGE=/d' tools/traffic_annotation/bin/README.md -grep '^CLANG_REVISION =' tools/clang/scripts/update.py >> tools/traffic_annotation/bin/README.md cat build/util/LASTCHANGE >> tools/traffic_annotation/bin/README.md git commit -a -m 'Roll traffic_annotation checkers' git cl upload @@ -39,10 +34,6 @@ git cl upload # On Windows: ```bash git new-branch roll_traffic_annotation_tools -python tools/clang/scripts/build.py --bootstrap ^ - --without-android --extra-tools traffic_annotation_extractor -cp third_party/llvm-build/Release+Asserts/bin/traffic_annotation_extractor.exe ^ - tools/traffic_annotation/bin/win32/ # These GN flags produce an optimized, stripped binary that has no dependency # on glib. @@ -55,13 +46,7 @@ cp -p out/Default/traffic_annotation_auditor.exe ^ python third_party/depot_tools/upload_to_google_storage.py ^ -b chromium-tools-traffic_annotation ^ tools/traffic_annotation/bin/win32/traffic_annotation_auditor.exe -python third_party/depot_tools/upload_to_google_storage.py ^ - -b chromium-tools-traffic_annotation ^ - tools/traffic_annotation/bin/win32/traffic_annotation_extractor.exe -sed -i "/^CLANG_REVISION =/d" tools/traffic_annotation/bin/README.md sed -i "/^LASTCHANGE=/d" tools/traffic_annotation/bin/README.md -grep "^CLANG_REVISION =" tools/clang/scripts/update.py >> ^ - tools/traffic_annotation/bin/README.md cat build/util/LASTCHANGE >> tools/traffic_annotation/bin/README.md dos2unix tools/traffic_annotation/bin/README.md git commit -a -m 'Roll traffic_annotation checkers' @@ -71,8 +56,7 @@ git cl upload and land the resulting CL. -The following two lines will be updated by the above script, and the modified +The following line will be updated by the above script, and the modified README should be committed along with the updated .sha1 checksums. -CLANG_REVISION = '64a362e7216a43e3ad44e50a89265e72aeb14294' LASTCHANGE=ae8f603e9821f418ed34b6e0908e69e350543207-refs/heads/master@{#736938} diff --git a/tools/traffic_annotation/bin/linux64/traffic_annotation_extractor.sha1 b/tools/traffic_annotation/bin/linux64/traffic_annotation_extractor.sha1 deleted file mode 100644 index ec1fc1e526dc44..00000000000000 --- a/tools/traffic_annotation/bin/linux64/traffic_annotation_extractor.sha1 +++ /dev/null @@ -1 +0,0 @@ -8618305abe807cbeefaaa512e47796f89116bf98 \ No newline at end of file diff --git a/tools/traffic_annotation/bin/win32/traffic_annotation_extractor.exe.sha1 b/tools/traffic_annotation/bin/win32/traffic_annotation_extractor.exe.sha1 deleted file mode 100644 index 49fa49b00fcac3..00000000000000 --- a/tools/traffic_annotation/bin/win32/traffic_annotation_extractor.exe.sha1 +++ /dev/null @@ -1 +0,0 @@ -4f467153df6ea1dee2d83efd8b01dc78673e88c3 \ No newline at end of file diff --git a/tools/traffic_annotation/scripts/README.md b/tools/traffic_annotation/scripts/README.md index ab53304e5cda18..9b7ff77b8fb1eb 100644 --- a/tools/traffic_annotation/scripts/README.md +++ b/tools/traffic_annotation/scripts/README.md @@ -8,7 +8,7 @@ are run in error resilient mode. Requires a compiled build directory to run. # traffic_annotation_auditor_tests.py Runs tests to ensure traffic_annotation_auditor is performing as expected. Tests include: - - Checking if auditor and underlying clang tool run, and an expected minimum + - Checking if auditor and underlying extractor run, and an expected minimum number of outputs are returned. - Checking if enabling or disabling heuristics that make tests faster has any effect on the output. diff --git a/tools/traffic_annotation/scripts/extractor.py b/tools/traffic_annotation/scripts/extractor.py index 69b135100a6559..b239ffc25dbe79 100755 --- a/tools/traffic_annotation/scripts/extractor.py +++ b/tools/traffic_annotation/scripts/extractor.py @@ -82,8 +82,8 @@ def parse_definition(self, re_match): self._parse_body(body) - def clang_tool_output_string(self): - """Returns a string formatted for clang-tool-style output.""" + def extractor_output_string(self): + """Returns a string formatted for output.""" return "\n".join(map(str, [ "==== NEW ANNOTATION ====", self.file_path, @@ -247,7 +247,7 @@ def main(): # Print output. for annotation in annotation_definitions: - print(annotation.clang_tool_output_string()) + print(annotation.extractor_output_string()) # If all files were successfully checked for annotations but none of them had # any, print something so that the traffic_annotation_auditor knows there was diff --git a/tools/traffic_annotation/scripts/test_data/valid_file.cc b/tools/traffic_annotation/scripts/test_data/valid_file.cc index fc03c18fbdcc2e..83b13d356aa217 100644 --- a/tools/traffic_annotation/scripts/test_data/valid_file.cc +++ b/tools/traffic_annotation/scripts/test_data/valid_file.cc @@ -6,8 +6,8 @@ #include "dummy_classes.h" -// This file provides samples for testing the extractor.py replacement script -// for the clang tool. +// This file provides samples for testing the extractor.py script. + namespace { net::NetworkTrafficAnnotationTag kTrafficAnnotation = diff --git a/tools/traffic_annotation/scripts/traffic_annotation_auditor_tests.py b/tools/traffic_annotation/scripts/traffic_annotation_auditor_tests.py index 14c2ed99e2a9b4..4a3d171b937d47 100755 --- a/tools/traffic_annotation/scripts/traffic_annotation_auditor_tests.py +++ b/tools/traffic_annotation/scripts/traffic_annotation_auditor_tests.py @@ -58,18 +58,15 @@ def CheckAuditorResults(self): [ "--test-only", "--error-resilient", - "--extractor-backend=python_script", ], # Failing on any runtime error. [ "--test-only", - "--extractor-backend=python_script", ], # No heuristic filtering. [ "--test-only", "--no-filtering", - "--extractor-backend=python_script", ], ]