Merge changes I7d5ab9fb,Ib34d291f am: 6091a2d602 am: c547fe5466

Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/1360880

Change-Id: I8a0812d1451b9c02f852a1920c25e8669ab733eb

8 files changed, 177 insertions, 27 deletions

diff --git a/cmds/lshal/Android.bp b/cmds/lshal/Android.bp
index 5afae4b7d3..987adafc8e 100644
--- a/cmds/lshal/Android.bp
+++ b/cmds/lshal/Android.bp
@@ -75,7 +75,7 @@ cc_test {
     defaults: ["lshal_defaults"],
     gtest: true,
     static_libs: [
-        "android.hardware.tests.baz@1.0",
+        "android.hardware.tests.inheritance@1.0",
         "libgmock",
     ],
     shared_libs: [
diff --git a/cmds/lshal/DebugCommand.cpp b/cmds/lshal/DebugCommand.cpp
index af22ac9b3d..72958bd2a9 100644
--- a/cmds/lshal/DebugCommand.cpp
+++ b/cmds/lshal/DebugCommand.cpp
@@ -39,7 +39,7 @@ Status DebugCommand::parseArgs(const Arg &arg) {
     // Optargs cannnot be used because the flag should not be considered set
     // if it should really be contained in mOptions.
     if (std::string(arg.argv[optind]) == "-E") {
-        mExcludesParentInstances = true;
+        mParentDebugInfoLevel = ParentDebugInfoLevel::NOTHING;
         optind++;
     }
 
@@ -67,7 +67,7 @@ Status DebugCommand::main(const Arg &arg) {
 
     return mLshal.emitDebugInfo(
             pair.first, pair.second.empty() ? "default" : pair.second, mOptions,
-            mExcludesParentInstances,
+            mParentDebugInfoLevel,
             mLshal.out().buf(),
             mLshal.err());
 }
diff --git a/cmds/lshal/DebugCommand.h b/cmds/lshal/DebugCommand.h
index cd57e31bfc..317cc2821c 100644
--- a/cmds/lshal/DebugCommand.h
+++ b/cmds/lshal/DebugCommand.h
@@ -21,6 +21,7 @@
 #include <android-base/macros.h>
 
 #include "Command.h"
+#include "ParentDebugInfoLevel.h"
 #include "utils.h"
 
 namespace android {
@@ -42,9 +43,8 @@ private:
     std::string mInterfaceName;
     std::vector<std::string> mOptions;
 
-    // Outputs the actual descriptor of a hal instead of the debug output
-    // if the arguments provided are a superclass of the actual hal impl.
-    bool mExcludesParentInstances;
+    // See comment on ParentDebugInfoLevel.
+    ParentDebugInfoLevel mParentDebugInfoLevel = ParentDebugInfoLevel::FULL;
 
     DISALLOW_COPY_AND_ASSIGN(DebugCommand);
 };
diff --git a/cmds/lshal/ListCommand.cpp b/cmds/lshal/ListCommand.cpp
index fb11cee33e..a805a4844a 100644
--- a/cmds/lshal/ListCommand.cpp
+++ b/cmds/lshal/ListCommand.cpp
@@ -555,7 +555,7 @@ void ListCommand::dumpTable(const NullableOStream<std::ostream>& out) const {
                 std::stringstream ss;
                 auto pair = splitFirst(iName, '/');
                 mLshal.emitDebugInfo(pair.first, pair.second, {},
-                                     false /* excludesParentInstances */, ss,
+                                     ParentDebugInfoLevel::FQNAME_ONLY, ss,
                                      NullableOStream<std::ostream>(nullptr));
                 return ss.str();
             };
diff --git a/cmds/lshal/Lshal.cpp b/cmds/lshal/Lshal.cpp
index 132b31ebc3..2c3efe524e 100644
--- a/cmds/lshal/Lshal.cpp
+++ b/cmds/lshal/Lshal.cpp
@@ -101,7 +101,7 @@ Status Lshal::emitDebugInfo(
         const std::string &interfaceName,
         const std::string &instanceName,
         const std::vector<std::string> &options,
-        bool excludesParentInstances,
+        ParentDebugInfoLevel parentDebugInfoLevel,
         std::ostream &out,
         NullableOStream<std::ostream> err) const {
     using android::hidl::base::V1_0::IBase;
@@ -126,7 +126,7 @@ Status Lshal::emitDebugInfo(
         return NO_INTERFACE;
     }
 
-    if (excludesParentInstances) {
+    if (parentDebugInfoLevel != ParentDebugInfoLevel::FULL) {
         const std::string descriptor = getDescriptor(base.get());
         if (descriptor.empty()) {
             std::string msg = interfaceName + "/" + instanceName + " getDescriptor failed";
@@ -134,6 +134,9 @@ Status Lshal::emitDebugInfo(
             LOG(ERROR) << msg;
         }
         if (descriptor != interfaceName) {
+            if (parentDebugInfoLevel == ParentDebugInfoLevel::FQNAME_ONLY) {
+                out << "[See " << descriptor << "/" << instanceName << "]";
+            }
             return OK;
         }
     }
diff --git a/cmds/lshal/Lshal.h b/cmds/lshal/Lshal.h
index 830bd872ff..50279d4d7a 100644
--- a/cmds/lshal/Lshal.h
+++ b/cmds/lshal/Lshal.h
@@ -25,6 +25,7 @@
 
 #include "Command.h"
 #include "NullableOStream.h"
+#include "ParentDebugInfoLevel.h"
 #include "utils.h"
 
 namespace android {
@@ -49,7 +50,7 @@ public:
             const std::string &interfaceName,
             const std::string &instanceName,
             const std::vector<std::string> &options,
-            bool excludesParentInstances,
+            ParentDebugInfoLevel parentDebugInfoLevel,
             std::ostream &out,
             NullableOStream<std::ostream> err) const;
 
diff --git a/cmds/lshal/ParentDebugInfoLevel.h b/cmds/lshal/ParentDebugInfoLevel.h
new file mode 100644
index 0000000000..12ac9c8648
--- /dev/null
+++ b/cmds/lshal/ParentDebugInfoLevel.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+
+namespace android {
+namespace lshal {
+
+// Describe verbosity when dumping debug information on a HAL service by
+// referring to a parent HAL interface FQName (for example, when dumping debug information
+// on foo@1.0::IFoo but the HAL implementation is foo@1.1::IFoo).
+enum class ParentDebugInfoLevel {
+    // Write nothing.
+    NOTHING,
+    // Write a short description that includes the FQName of the real implementation.
+    FQNAME_ONLY,
+    // Write full debug info.
+    FULL,
+};
+
+} // namespace lshal
+} // namespace android
diff --git a/cmds/lshal/test.cpp b/cmds/lshal/test.cpp
index 3d550babf4..afe5d63575 100644
--- a/cmds/lshal/test.cpp
+++ b/cmds/lshal/test.cpp
@@ -24,7 +24,7 @@
 
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
-#include <android/hardware/tests/baz/1.0/IQuux.h>
+#include <android/hardware/tests/inheritance/1.0/IChild.h>
 #include <hidl/HidlTransportSupport.h>
 #include <vintf/parse_xml.h>
 
@@ -44,6 +44,7 @@ using ::android::hardware::hidl_death_recipient;
 using ::android::hardware::hidl_handle;
 using ::android::hardware::hidl_string;
 using ::android::hardware::hidl_vec;
+using ::android::hardware::Void;
 using android::vintf::Arch;
 using android::vintf::CompatibilityMatrix;
 using android::vintf::gCompatibilityMatrixConverter;
@@ -59,10 +60,14 @@ using hidl_hash = hidl_array<uint8_t, 32>;
 namespace android {
 namespace hardware {
 namespace tests {
-namespace baz {
+namespace inheritance {
 namespace V1_0 {
 namespace implementation {
-struct Quux : android::hardware::tests::baz::V1_0::IQuux {
+struct Child : android::hardware::tests::inheritance::V1_0::IChild {
+    ::android::hardware::Return<void> doChild() override { return Void(); }
+    ::android::hardware::Return<void> doParent() override { return Void(); }
+    ::android::hardware::Return<void> doGrandparent() override { return Void(); }
+
     ::android::hardware::Return<void> debug(const hidl_handle& hh, const hidl_vec<hidl_string>& options) override {
         const native_handle_t *handle = hh.getNativeHandle();
         if (handle->numFds < 1) {
@@ -76,7 +81,7 @@ struct Quux : android::hardware::tests::baz::V1_0::IQuux {
         }
         ssize_t written = write(fd, content.c_str(), content.size());
         if (written != (ssize_t)content.size()) {
-            LOG(WARNING) << "SERVER(Quux) debug writes " << written << " bytes < "
+            LOG(WARNING) << "SERVER(Child) debug writes " << written << " bytes < "
                     << content.size() << " bytes, errno = " << errno;
         }
         return Void();
@@ -85,7 +90,7 @@ struct Quux : android::hardware::tests::baz::V1_0::IQuux {
 
 } // namespace implementation
 } // namespace V1_0
-} // namespace baz
+} // namespace inheritance
 } // namespace tests
 } // namespace hardware
 
@@ -124,18 +129,24 @@ public:
 class DebugTest : public ::testing::Test {
 public:
     void SetUp() override {
-        using ::android::hardware::tests::baz::V1_0::IQuux;
-        using ::android::hardware::tests::baz::V1_0::implementation::Quux;
+        using ::android::hardware::tests::inheritance::V1_0::IChild;
+        using ::android::hardware::tests::inheritance::V1_0::IParent;
+        using ::android::hardware::tests::inheritance::V1_0::IGrandparent;
+        using ::android::hardware::tests::inheritance::V1_0::implementation::Child;
 
         err.str("");
         out.str("");
         serviceManager = new testing::NiceMock<MockServiceManager>();
-        ON_CALL(*serviceManager, get(_, _)).WillByDefault(Invoke(
-            [](const auto &iface, const auto &inst) -> ::android::hardware::Return<sp<IBase>> {
-                if (iface == IQuux::descriptor && inst == "default")
-                    return new Quux();
-                return nullptr;
-            }));
+        ON_CALL(*serviceManager, get(_, _))
+                .WillByDefault(
+                        Invoke([](const auto& iface,
+                                  const auto& inst) -> ::android::hardware::Return<sp<IBase>> {
+                            if (inst != "default") return nullptr;
+                            if (iface == IChild::descriptor || iface == IParent::descriptor ||
+                                iface == IGrandparent::descriptor)
+                                return new Child();
+                            return nullptr;
+                        }));
 
         lshal = std::make_unique<Lshal>(out, err, serviceManager, serviceManager);
     }
@@ -159,17 +170,17 @@ static Status callMain(const std::unique_ptr<T>& lshal, const std::vector<const
 
 TEST_F(DebugTest, Debug) {
     EXPECT_EQ(0u, callMain(lshal, {
-        "lshal", "debug", "android.hardware.tests.baz@1.0::IQuux/default", "foo", "bar"
+        "lshal", "debug", "android.hardware.tests.inheritance@1.0::IChild/default", "foo", "bar"
     }));
-    EXPECT_THAT(out.str(), StrEq("android.hardware.tests.baz@1.0::IQuux\nfoo\nbar"));
+    EXPECT_THAT(out.str(), StrEq("android.hardware.tests.inheritance@1.0::IChild\nfoo\nbar"));
     EXPECT_THAT(err.str(), IsEmpty());
 }
 
 TEST_F(DebugTest, Debug2) {
     EXPECT_EQ(0u, callMain(lshal, {
-        "lshal", "debug", "android.hardware.tests.baz@1.0::IQuux", "baz", "quux"
+        "lshal", "debug", "android.hardware.tests.inheritance@1.0::IChild", "baz", "quux"
     }));
-    EXPECT_THAT(out.str(), StrEq("android.hardware.tests.baz@1.0::IQuux\nbaz\nquux"));
+    EXPECT_THAT(out.str(), StrEq("android.hardware.tests.inheritance@1.0::IChild\nbaz\nquux"));
     EXPECT_THAT(err.str(), IsEmpty());
 }
 
@@ -180,6 +191,22 @@ TEST_F(DebugTest, Debug3) {
     EXPECT_THAT(err.str(), HasSubstr("does not exist"));
 }
 
+TEST_F(DebugTest, DebugParent) {
+    EXPECT_EQ(0u, callMain(lshal, {
+        "lshal", "debug", "android.hardware.tests.inheritance@1.0::IParent", "calling parent"
+    }));
+    EXPECT_THAT(out.str(), StrEq("android.hardware.tests.inheritance@1.0::IChild\ncalling parent"));
+    EXPECT_THAT(err.str(), IsEmpty());
+}
+
+TEST_F(DebugTest, DebugParentExclude) {
+    EXPECT_EQ(0u, callMain(lshal, {
+        "lshal", "debug", "-E", "android.hardware.tests.inheritance@1.0::IParent", "excluding"
+    }));
+    EXPECT_THAT(out.str(), IsEmpty());
+    EXPECT_THAT(err.str(), IsEmpty());
+}
+
 class MockLshal : public Lshal {
 public:
     MockLshal() {}
@@ -766,6 +793,91 @@ TEST_F(ListTest, Vintf) {
     EXPECT_EQ("", err.str());
 }
 
+// Fake service returned by mocked IServiceManager::get for DumpDebug.
+// The interfaceChain and getHashChain functions returns
+// foo(id - 1) -> foo(id - 2) -> ... foo1 -> IBase.
+class InheritingService : public IBase {
+public:
+    explicit InheritingService(pid_t id) : mId(id) {}
+    android::hardware::Return<void> interfaceDescriptor(interfaceDescriptor_cb cb) override {
+        cb(getInterfaceName(mId));
+        return hardware::Void();
+    }
+    android::hardware::Return<void> interfaceChain(interfaceChain_cb cb) override {
+        std::vector<hidl_string> ret;
+        for (auto i = mId; i > 0; --i) {
+            ret.push_back(getInterfaceName(i));
+        }
+        ret.push_back(IBase::descriptor);
+        cb(ret);
+        return hardware::Void();
+    }
+    android::hardware::Return<void> getHashChain(getHashChain_cb cb) override {
+        std::vector<hidl_hash> ret;
+        for (auto i = mId; i > 0; --i) {
+            ret.push_back(getHashFromId(i));
+        }
+        ret.push_back(getHashFromId(0xff));
+        cb(ret);
+        return hardware::Void();
+    }
+    android::hardware::Return<void> debug(const hidl_handle& hh,
+                                          const hidl_vec<hidl_string>&) override {
+        const native_handle_t* handle = hh.getNativeHandle();
+        if (handle->numFds < 1) {
+            return Void();
+        }
+        int fd = handle->data[0];
+        std::string content = "debug info for ";
+        content += getInterfaceName(mId);
+        ssize_t written = write(fd, content.c_str(), content.size());
+        if (written != (ssize_t)content.size()) {
+            LOG(WARNING) << "SERVER(" << descriptor << ") debug writes " << written << " bytes < "
+                         << content.size() << " bytes, errno = " << errno;
+        }
+        return Void();
+    }
+
+private:
+    pid_t mId;
+};
+
+TEST_F(ListTest, DumpDebug) {
+    size_t inheritanceLevel = 3;
+    sp<IBase> service = new InheritingService(inheritanceLevel);
+
+    EXPECT_CALL(*serviceManager, list(_)).WillRepeatedly(Invoke([&](IServiceManager::list_cb cb) {
+        std::vector<hidl_string> ret;
+        for (auto i = 1; i <= inheritanceLevel; ++i) {
+            ret.push_back(getInterfaceName(i) + "/default");
+        }
+        cb(ret);
+        return hardware::Void();
+    }));
+    EXPECT_CALL(*serviceManager, get(_, _))
+            .WillRepeatedly(
+                    Invoke([&](const hidl_string&, const hidl_string& instance) -> sp<IBase> {
+                        int id = getIdFromInstanceName(instance);
+                        if (id > inheritanceLevel) return nullptr;
+                        return sp<IBase>(service);
+                    }));
+
+    const std::string expected = "[fake description 0]\n"
+                                 "Interface\n"
+                                 "a.h.foo1@1.0::IFoo/default\n"
+                                 "[See a.h.foo3@3.0::IFoo/default]\n"
+                                 "a.h.foo2@2.0::IFoo/default\n"
+                                 "[See a.h.foo3@3.0::IFoo/default]\n"
+                                 "a.h.foo3@3.0::IFoo/default\n"
+                                 "debug info for a.h.foo3@3.0::IFoo\n"
+                                 "\n";
+
+    optind = 1; // mimic Lshal::parseArg()
+    EXPECT_EQ(0u, mockList->main(createArg({"lshal", "--types=b", "-id"})));
+    EXPECT_EQ(expected, out.str());
+    EXPECT_EQ("", err.str());
+}
+
 class ListVintfTest : public ListTest {
 public:
     virtual void SetUp() override {