Remove hiddenapi access flags in FixedUpDexFile

The hiddenapi tool will mess with the access flags of fields and
methods in order to record which are '@hide'. We need to undo this
before passing any dex files down to jvmti agents.

Test: ./test.py --host -j50
Bug: 72550707
Bug: 64382372

Change-Id: Ibc9a96a6b541c06844f276db009ac29514f7a3bb
diff --git a/openjdkjvmti/fixed_up_dex_file.cc b/openjdkjvmti/fixed_up_dex_file.cc
index 6c66f12..a8d2a37 100644
--- a/openjdkjvmti/fixed_up_dex_file.cc
+++ b/openjdkjvmti/fixed_up_dex_file.cc
@@ -33,6 +33,7 @@
 #include "dex/art_dex_file_loader.h"
 #include "dex/dex_file-inl.h"
 #include "dex/dex_file_loader.h"
+#include "dex/dex_file_verifier.h"
 
 // Runtime includes.
 #include "dex_container.h"
@@ -67,6 +68,20 @@
   vdex->UnquickenDexFile(new_dex_file, original_dex_file, /* decompile_return_instruction */true);
 }
 
+static void DCheckVerifyDexFile(const art::DexFile& dex) {
+  if (art::kIsDebugBuild) {
+    std::string error;
+    if (!art::DexFileVerifier::Verify(&dex,
+                                      dex.Begin(),
+                                      dex.Size(),
+                                      "FixedUpDexFile_Verification.dex",
+                                      /*verify_checksum*/ true,
+                                      &error)) {
+      LOG(FATAL) << "Failed to verify de-quickened dex file: " << error;
+    }
+  }
+}
+
 std::unique_ptr<FixedUpDexFile> FixedUpDexFile::Create(const art::DexFile& original,
                                                        const char* descriptor) {
   // Copy the data into mutable memory.
@@ -121,6 +136,7 @@
   DoDexUnquicken(*new_dex_file, original);
 
   RecomputeDexChecksum(const_cast<art::DexFile*>(new_dex_file.get()));
+  DCheckVerifyDexFile(*new_dex_file);
   std::unique_ptr<FixedUpDexFile> ret(new FixedUpDexFile(std::move(new_dex_file), std::move(data)));
   return ret;
 }
diff --git a/runtime/leb128.h b/runtime/leb128.h
index 9fb09d8..07eadc1 100644
--- a/runtime/leb128.h
+++ b/runtime/leb128.h
@@ -55,6 +55,10 @@
   return static_cast<uint32_t>(result);
 }
 
+static inline uint32_t DecodeUnsignedLeb128WithoutMovingCursor(const uint8_t* data) {
+  return DecodeUnsignedLeb128(&data);
+}
+
 static inline bool DecodeUnsignedLeb128Checked(const uint8_t** data,
                                                const void* end,
                                                uint32_t* out) {
@@ -203,6 +207,34 @@
   return (x * 37) >> 8;
 }
 
+static inline bool IsLeb128Terminator(const uint8_t* ptr) {
+  return *ptr <= 0x7f;
+}
+
+// Returns the first byte of a Leb128 value assuming that:
+// (1) `end_ptr` points to the first byte after the Leb128 value, and
+// (2) there is another Leb128 value before this one.
+template <typename T>
+static inline T* ReverseSearchUnsignedLeb128(T* end_ptr) {
+  static_assert(std::is_same<typename std::remove_const<T>::type, uint8_t>::value,
+                "T must be a uint8_t");
+  T* ptr = end_ptr;
+
+  // Move one byte back, check that this is the terminating byte.
+  ptr--;
+  DCHECK(IsLeb128Terminator(ptr));
+
+  // Keep moving back while the previous byte is not a terminating byte.
+  // Fail after reading five bytes in case there isn't another Leb128 value
+  // before this one.
+  while (!IsLeb128Terminator(ptr - 1)) {
+    ptr--;
+    DCHECK_LE(static_cast<ptrdiff_t>(end_ptr - ptr), 5);
+  }
+
+  return ptr;
+}
+
 // Returns the number of bytes needed to encode the value in unsigned LEB128.
 static inline uint32_t SignedLeb128Size(int32_t data) {
   // Like UnsignedLeb128Size(), but we need one bit beyond the highest bit that differs from sign.
diff --git a/runtime/vdex_file.cc b/runtime/vdex_file.cc
index 36ebb17..7428e98 100644
--- a/runtime/vdex_file.cc
+++ b/runtime/vdex_file.cc
@@ -30,6 +30,8 @@
 #include "dex/dex_file.h"
 #include "dex/dex_file_loader.h"
 #include "dex_to_dex_decompiler.h"
+#include "hidden_api_access_flags.h"
+#include "leb128.h"
 #include "quicken_info.h"
 
 namespace art {
@@ -262,6 +264,18 @@
   UnquickenDexFile(target_dex_file, source_dex_file.Begin(), decompile_return_instruction);
 }
 
+static void UpdateAccessFlags(uint8_t* data, uint32_t new_flag, bool is_method) {
+  // Go back 1 uleb to start.
+  data = ReverseSearchUnsignedLeb128(data);
+  if (is_method) {
+    // Methods have another uleb field before the access flags
+    data = ReverseSearchUnsignedLeb128(data);
+  }
+  DCHECK_EQ(HiddenApiAccessFlags::RemoveFromDex(DecodeUnsignedLeb128WithoutMovingCursor(data)),
+            new_flag);
+  UpdateUnsignedLeb128(data, new_flag);
+}
+
 void VdexFile::UnquickenDexFile(const DexFile& target_dex_file,
                                 const uint8_t* source_dex_begin,
                                 bool decompile_return_instruction) const {
@@ -280,27 +294,32 @@
       for (ClassDataItemIterator class_it(target_dex_file, class_data);
            class_it.HasNext();
            class_it.Next()) {
-        if (class_it.IsAtMethod() && class_it.GetMethodCodeItem() != nullptr) {
+        if (class_it.IsAtMethod()) {
           const DexFile::CodeItem* code_item = class_it.GetMethodCodeItem();
-          if (!unquickened_code_item.emplace(code_item).second) {
-            // Already unquickened this code item, do not do it again.
-            continue;
+          if (code_item != nullptr && unquickened_code_item.emplace(code_item).second) {
+            ArrayRef<const uint8_t> quicken_data;
+            if (!quickening_info.empty()) {
+              const uint32_t quickening_offset = GetQuickeningInfoOffset(
+                  GetQuickenInfoOffsetTable(source_dex_begin,
+                                            target_dex_file.NumMethodIds(),
+                                            quickening_info),
+                  class_it.GetMemberIndex(),
+                  quickening_info);
+              quicken_data = GetQuickeningInfoAt(quickening_info, quickening_offset);
+            }
+            optimizer::ArtDecompileDEX(
+                target_dex_file,
+                *code_item,
+                quicken_data,
+                decompile_return_instruction);
           }
-          ArrayRef<const uint8_t> quicken_data;
-          if (!quickening_info.empty()) {
-            const uint32_t quickening_offset = GetQuickeningInfoOffset(
-                GetQuickenInfoOffsetTable(source_dex_begin,
-                                          target_dex_file.NumMethodIds(),
-                                          quickening_info),
-                class_it.GetMemberIndex(),
-                quickening_info);
-            quicken_data = GetQuickeningInfoAt(quickening_info, quickening_offset);
-          }
-          optimizer::ArtDecompileDEX(
-              target_dex_file,
-              *code_item,
-              quicken_data,
-              decompile_return_instruction);
+          UpdateAccessFlags(const_cast<uint8_t*>(class_it.DataPointer()),
+                            class_it.GetMemberAccessFlags(),
+                            /*is_method*/ true);
+        } else {
+          UpdateAccessFlags(const_cast<uint8_t*>(class_it.DataPointer()),
+                            class_it.GetMemberAccessFlags(),
+                            /*is_method*/ false);
         }
       }
     }
diff --git a/test/983-source-transform-verify/expected.txt b/test/983-source-transform-verify/expected.txt
index abcdf3a..aa51ea0 100644
--- a/test/983-source-transform-verify/expected.txt
+++ b/test/983-source-transform-verify/expected.txt
@@ -1,2 +1,3 @@
 Dex file hook for art/Test983$Transform
 Dex file hook for java/lang/Object
+Dex file hook for java/lang/ClassLoader
diff --git a/test/983-source-transform-verify/src/art/Test983.java b/test/983-source-transform-verify/src/art/Test983.java
index b81e7f4..faae96a 100644
--- a/test/983-source-transform-verify/src/art/Test983.java
+++ b/test/983-source-transform-verify/src/art/Test983.java
@@ -16,7 +16,6 @@
 
 package art;
 
-import java.util.Base64;
 public class Test983 {
   static class Transform {
     public void sayHi() {
@@ -29,10 +28,11 @@
   }
 
   public static void doTest() {
-    Transform abc = new Transform();
     Redefinition.enableCommonRetransformation(true);
     Redefinition.doCommonClassRetransformation(Transform.class);
     Redefinition.doCommonClassRetransformation(Object.class);
+    // NB java.lang.ClassLoader has hidden fields.
+    Redefinition.doCommonClassRetransformation(ClassLoader.class);
     Redefinition.enableCommonRetransformation(false);
   }
 }
diff --git a/tools/hiddenapi/hiddenapi.cc b/tools/hiddenapi/hiddenapi.cc
index a755fdb..c893da6 100644
--- a/tools/hiddenapi/hiddenapi.cc
+++ b/tools/hiddenapi/hiddenapi.cc
@@ -118,9 +118,11 @@
     // until we hit the terminating byte of the previous Leb128 value.
     const uint8_t* ptr = it_.DataPointer();
     if (it_.IsAtMethod()) {
-      ptr = ReverseSearchUnsignedLeb128(ptr, it_.GetMethodCodeItemOffset());
+      ptr = ReverseSearchUnsignedLeb128(ptr);
+      DCHECK_EQ(DecodeUnsignedLeb128WithoutMovingCursor(ptr), it_.GetMethodCodeItemOffset());
     }
-    ptr = ReverseSearchUnsignedLeb128(ptr, old_flags);
+    ptr = ReverseSearchUnsignedLeb128(ptr);
+    DCHECK_EQ(DecodeUnsignedLeb128WithoutMovingCursor(ptr), old_flags);
 
     // Overwrite the access flags.
     UpdateUnsignedLeb128(const_cast<uint8_t*>(ptr), new_flags);
@@ -158,38 +160,6 @@
     return klass_.GetDexFile().GetFieldId(it_.GetMemberIndex());
   }
 
-  static inline bool IsLeb128Terminator(const uint8_t* ptr) {
-    return *ptr <= 0x7f;
-  }
-
-  // Returns the first byte of a Leb128 value assuming that:
-  // (1) `end_ptr` points to the first byte after the Leb128 value, and
-  // (2) there is another Leb128 value before this one.
-  // The function will fail after reading 5 bytes (the longest supported Leb128
-  // encoding) to protect against situations when (2) is not satisfied.
-  // When a Leb128 value is discovered, it is decoded and CHECKed against `value`.
-  static const uint8_t* ReverseSearchUnsignedLeb128(const uint8_t* end_ptr, uint32_t expected) {
-    const uint8_t* ptr = end_ptr;
-
-    // Move one byte back, check that this is the terminating byte.
-    ptr--;
-    CHECK(IsLeb128Terminator(ptr));
-
-    // Keep moving back while the previous byte is not a terminating byte.
-    // Fail after reading five bytes in case there isn't another Leb128 value
-    // before this one.
-    while (!IsLeb128Terminator(ptr - 1)) {
-      ptr--;
-      CHECK_LE((size_t) (end_ptr - ptr), 5u);
-    }
-
-    // Check that the decoded value matches the `expected` value.
-    const uint8_t* tmp_ptr = ptr;
-    CHECK_EQ(DecodeUnsignedLeb128(&tmp_ptr), expected);
-
-    return ptr;
-  }
-
   const DexClass& klass_;
   const ClassDataItemIterator& it_;
 };