[llvm] [llvm][ELF]Add Shdr check for getBuildID (PR #126537)

James Henderson via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 28 00:42:39 PST 2025


================
@@ -0,0 +1,61 @@
+//===- BuildIDTest.cpp - Tests for getBuildID ----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Object/BuildID.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/ObjectYAML/yaml2obj.h"
+#include "llvm/Support/YAMLTraits.h"
+#include "llvm/Testing/Support/Error.h"
+
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace llvm::object;
+
+template <class ELFT>
+static Expected<ELFObjectFile<ELFT>> toBinary(SmallVectorImpl<char> &Storage,
+                                              StringRef Yaml) {
+  raw_svector_ostream OS(Storage);
+  yaml::Input YIn(Yaml);
+  if (!yaml::convertYAML(YIn, OS, [](const Twine &Msg) {}))
+    return createStringError(std::errc::invalid_argument,
+                             "unable to convert YAML");
+  return ELFObjectFile<ELFT>::create(MemoryBufferRef(OS.str(), "dummyELF"));
+}
+
+TEST(BuildIDTest, InvalidNoteFileSizeTest) {
----------------
jh7370 wrote:

> I’m not sure if this aligns with your idea, but I’ve implemented it in the latest commit. Let me know if I misunderstood anything.

Looks perfect (barring some naming nits), thanks!

On the note of naming, you should probably change the name of `InvalidNoteFileSizeTest` to clarify the specific case this case is testing.

> If I did not misunderstand, I should know the generated ELF file size before I generate it? I think the file size may change according to yaml2obj.
Please let me know if I misunderstood anything...

You didn't misunderstand anything, but I think it's still worth it. Firstly, it's rare that yaml2obj will change how an ELF is generated, as the tool is quite stable now and the rules on how to line out an object are fairly well defined. Secondly, it's worth it to make sure we don't have e.g. an incorrect `<` versus `<=` comparison or similar. By adding a test case that passes with it being exactly the right offset and another that fails when it is one off the right offset, we also can see if the edge moves in either direction. Let me know if you have any difficulties.

https://github.com/llvm/llvm-project/pull/126537


More information about the llvm-commits mailing list