https://github.com/JDevlieghere closed
https://github.com/llvm/llvm-project/pull/90099
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/JDevlieghere approved this pull request.
Looks good to me as well.
https://github.com/llvm/llvm-project/pull/90099
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://github.com/labath approved this pull request.
https://github.com/llvm/llvm-project/pull/90099
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/al45tair updated
https://github.com/llvm/llvm-project/pull/90099
>From ce54a7fb339a00029da266c9f518e344aac5d19e Mon Sep 17 00:00:00 2001
From: Alastair Houghton
Date: Thu, 25 Apr 2024 11:35:55 +0100
Subject: [PATCH 1/5] [LLDB][ELF] Fix section unification to not just use
al45tair wrote:
@labath @JDevlieghere Are we happy with this PR now? If so, I'll cherry pick it
to the various Apple forks; I need it in order to merge
https://github.com/apple/swift/pull/72061, which we need for both my fully
statically linked Swift SDK work and for Amazon Linux 2023 support
@@ -1854,6 +1854,49 @@ class VMAddressProvider {
};
}
+namespace {
+ // We have to do this because ELF doesn't have section IDs, and also
+ // doesn't require section names to be unique. (We use the section index
+ // for section IDs, but that isn't guaranteed to be the
https://github.com/al45tair updated
https://github.com/llvm/llvm-project/pull/90099
>From ce54a7fb339a00029da266c9f518e344aac5d19e Mon Sep 17 00:00:00 2001
From: Alastair Houghton
Date: Thu, 25 Apr 2024 11:35:55 +0100
Subject: [PATCH 1/5] [LLDB][ELF] Fix section unification to not just use
labath wrote:
> > I saw that, but a textual test is definitely preferable, particularly after
> > the linux xz fiasco. This wouldn't be the first test binary in our repo,
> > but in this case, I think it actually adds a lot of value, since yaml2obj
> > operates on the same level as what you
@@ -1854,6 +1854,49 @@ class VMAddressProvider {
};
}
+namespace {
+ // We have to do this because ELF doesn't have section IDs, and also
+ // doesn't require section names to be unique. (We use the section index
+ // for section IDs, but that isn't guaranteed to be the
https://github.com/al45tair updated
https://github.com/llvm/llvm-project/pull/90099
>From ce54a7fb339a00029da266c9f518e344aac5d19e Mon Sep 17 00:00:00 2001
From: Alastair Houghton
Date: Thu, 25 Apr 2024 11:35:55 +0100
Subject: [PATCH 1/4] [LLDB][ELF] Fix section unification to not just use
al45tair wrote:
> I saw that, but a textual test is definitely preferable, particularly after
> the linux xz fiasco. This wouldn't be the first test binary in our repo, but
> in this case, I think it actually adds a lot of value, since yaml2obj
> operates on the same level as what you are
labath wrote:
> > [this](https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/yaml2obj/ELF/duplicate-section-names.yaml)
> > test seems to indicate that's possible (the trick appears to me in `(n)`
> > name tag suffixes). Can you give that a shot?
>
> I've actually just included a
https://github.com/al45tair edited
https://github.com/llvm/llvm-project/pull/90099
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/al45tair updated
https://github.com/llvm/llvm-project/pull/90099
>From ce54a7fb339a00029da266c9f518e344aac5d19e Mon Sep 17 00:00:00 2001
From: Alastair Houghton
Date: Thu, 25 Apr 2024 11:35:55 +0100
Subject: [PATCH 1/3] [LLDB][ELF] Fix section unification to not just use
al45tair wrote:
> [this](https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/yaml2obj/ELF/duplicate-section-names.yaml)
> test seems to indicate that's possible (the trick appears to me in `(n)`
> name tag suffixes). Can you give that a shot?
I've actually just included a binary
@@ -1854,6 +1854,49 @@ class VMAddressProvider {
};
}
+namespace {
+ // We have to do this because ELF doesn't have section IDs, and also
+ // doesn't require section names to be unique. (We use the section index
+ // for section IDs, but that isn't guaranteed to be the
@@ -1854,6 +1854,49 @@ class VMAddressProvider {
};
}
+namespace {
+ // We have to do this because ELF doesn't have section IDs, and also
+ // doesn't require section names to be unique. (We use the section index
+ // for section IDs, but that isn't guaranteed to be the
@@ -1854,6 +1854,49 @@ class VMAddressProvider {
};
}
+namespace {
+ // We have to do this because ELF doesn't have section IDs, and also
+ // doesn't require section names to be unique. (We use the section index
+ // for section IDs, but that isn't guaranteed to be the
labath wrote:
> > Can we add a test for this with obj2yaml?
>
> Not sure what you're thinking here. We can't use `yaml2obj` to generate an
> object with this problem because that tool assumes symbols are in a uniquely
> named section
https://github.com/al45tair updated
https://github.com/llvm/llvm-project/pull/90099
>From ce54a7fb339a00029da266c9f518e344aac5d19e Mon Sep 17 00:00:00 2001
From: Alastair Houghton
Date: Thu, 25 Apr 2024 11:35:55 +0100
Subject: [PATCH 1/2] [LLDB][ELF] Fix section unification to not just use
https://github.com/al45tair updated
https://github.com/llvm/llvm-project/pull/90099
>From ce54a7fb339a00029da266c9f518e344aac5d19e Mon Sep 17 00:00:00 2001
From: Alastair Houghton
Date: Thu, 25 Apr 2024 11:35:55 +0100
Subject: [PATCH 1/2] [LLDB][ELF] Fix section unification to not just use
al45tair wrote:
> Can we add a test for this with obj2yaml?
Not sure what you're thinking here. We can't use `yaml2obj` to generate an
object with this problem because that tool assumes symbols are in a uniquely
named section (which is mostly reasonable), and `obj2yaml` doesn't exercise the
@@ -2067,10 +2110,8 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab,
user_id_t start_id,
SectionList *module_section_list =
module_sp ? module_sp->GetSectionList() : nullptr;
- // Local cache to avoid doing a FindSectionByName for each symbol. The "const
-
@@ -1854,6 +1854,49 @@ class VMAddressProvider {
};
}
+namespace {
+ // We have to do this because ELF doesn't have section IDs, and also
+ // doesn't require section names to be unique. (We use the section index
+ // for section IDs, but that isn't guaranteed to be the
@@ -1854,6 +1854,49 @@ class VMAddressProvider {
};
}
+namespace {
+ // We have to do this because ELF doesn't have section IDs, and also
+ // doesn't require section names to be unique. (We use the section index
+ // for section IDs, but that isn't guaranteed to be the
@@ -1854,6 +1854,49 @@ class VMAddressProvider {
};
}
+namespace {
al45tair wrote:
This file already uses anonymous namespaces, but I'll change the function to
`static` instead.
https://github.com/llvm/llvm-project/pull/90099
@@ -2067,10 +2110,8 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab,
user_id_t start_id,
SectionList *module_section_list =
module_sp ? module_sp->GetSectionList() : nullptr;
- // Local cache to avoid doing a FindSectionByName for each symbol. The "const
-
@@ -1854,6 +1854,49 @@ class VMAddressProvider {
};
}
+namespace {
JDevlieghere wrote:
Nit: LLVM and LLDB prefer static functions over anonymous namespaces.
https://github.com/llvm/llvm-project/pull/90099
___
https://github.com/JDevlieghere commented:
Can we add a test for this with obj2yaml?
https://github.com/llvm/llvm-project/pull/90099
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
@@ -1854,6 +1854,49 @@ class VMAddressProvider {
};
}
+namespace {
+ // We have to do this because ELF doesn't have section IDs, and also
+ // doesn't require section names to be unique. (We use the section index
+ // for section IDs, but that isn't guaranteed to be the
@@ -1854,6 +1854,49 @@ class VMAddressProvider {
};
}
+namespace {
+ // We have to do this because ELF doesn't have section IDs, and also
+ // doesn't require section names to be unique. (We use the section index
+ // for section IDs, but that isn't guaranteed to be the
https://github.com/JDevlieghere edited
https://github.com/llvm/llvm-project/pull/90099
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff f5953f46aa0a664461584b78c14cb141a3be2b9d
ce54a7fb339a00029da266c9f518e344aac5d19e --
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Alastair Houghton (al45tair)
Changes
Section unification cannot just use names, because it's valid for ELF binaries
to have multiple sections with the same name. We should check other section
properties too.
Fixes #88001.
https://github.com/al45tair created
https://github.com/llvm/llvm-project/pull/90099
Section unification cannot just use names, because it's valid for ELF binaries
to have multiple sections with the same name. We should check other section
properties too.
Fixes #88001.
rdar://124467787
35 matches
Mail list logo
