commit b7c47473b4c8271dbe361b18afb0de7408d0286d
author Steven Moreland <smoreland@google.com> Wed Nov 20 10:52:40 2019 -0800
committer Steven Moreland <smoreland@google.com> Wed Nov 20 13:39:38 2019 -0800
tree 559daf48a0044760c85126f1ba13be7d9f6289a9
parent 54618b461d9090610c8215b3882a8c3342caa8bb

Only addImportedNamesGranular when success.

Before, 'hidl-gen -v -Lcheck android.hardware.authsecret@1.0' was
segfaulting. This is because the '-v' check for unreferenced types is
getting the ASTs for all imported granular types. However, authsecret
has no types.hal file. Because addImportedNamesGranular was called even
on unsuccessful imports (which is, AFAIK, only ever accepted as a
non-error when implicitly importing types), this routine was trying to
access data in a null parsed types AST.

Bug: N/A
Test: source scripts/hal-queries.sh && hidl-gen -v -Lcheck $(aosp-interfaces)
Test: ./test/run_all_host_tests.sh
Change-Id: I96f655c3168b470c1a61f9feb783c2452b5101a5

AST.cpp

diff --git a/AST.cpp b/AST.cpp
index a4f7ef9..4eba768 100644
--- a/AST.cpp
+++ b/AST.cpp
@@ -387,13 +387,13 @@
         }
 
         for (const auto& subFQName : packageInterfaces) {
-            addToImportedNamesGranular(subFQName);
-
             // Do not enforce restrictions on imports.
             AST* ast = mCoordinator->parse(subFQName, &mImportedASTs, Coordinator::Enforce::NONE);
             if (ast == nullptr) {
                 return false;
             }
+            addToImportedNamesGranular(subFQName);
+
             // all previous single type imports are ignored.
             mImportedTypes.erase(ast);
         }
@@ -401,8 +401,6 @@
         return true;
     }
 
-    addToImportedNamesGranular(fqName);
-
     // cases like android.hardware.foo@1.0::IFoo.Internal
     //            android.hardware.foo@1.0::Abc.Internal
 
@@ -424,6 +422,7 @@
             // cases like android.hardware.foo@1.0::IFoo
             //        and android.hardware.foo@1.0::types
             mImportedTypes.erase(importAST);
+            addToImportedNamesGranular(fqName);
             return true;
         }
 
@@ -436,6 +435,7 @@
         }
         // will automatically create a set if it does not exist
         mImportedTypes[importAST].insert(match);
+        addToImportedNamesGranular(fqName);
         return true;
     }
 
@@ -454,6 +454,7 @@
         }
         // will automatically create a set if not exist
         mImportedTypes[importAST].insert(match);
+        addToImportedNamesGranular(fqName);
         return true;
     }
 
@@ -826,6 +827,9 @@
             AST* ast = mCoordinator->parse(
                     fqName, nullptr /* imported */, Coordinator::Enforce::NONE);
 
+            // imported names must have already been validated
+            CHECK(ast != nullptr) << fqName.string();
+
             ast->addDefinedTypes(allImportNames);
         } else {
             allImportNames->insert(fqName);