Le mer. 29 nov. 2023 à 13:21, Martin Storsjö <mar...@martin.st> a écrit :
>
> On Wed, 29 Nov 2023, Antonin Décimo wrote:
>
> > Le mer. 29 nov. 2023 à 12:32, Martin Storsjö <mar...@martin.st> a écrit :
> >>
> >> On Wed, 29 Nov 2023, Antonin Décimo wrote:
> >>
> >> > Signed-off-by: Antonin Décimo <anto...@tarides.com>
> >> > ---
> >> > mingw-w64-libraries/winpthreads/src/thread.c | 18 ++++++++++++++++--
> >> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/mingw-w64-libraries/winpthreads/src/thread.c
> >> > b/mingw-w64-libraries/winpthreads/src/thread.c
> >> > index 7c5e75f91..16b13b07c 100644
> >> > --- a/mingw-w64-libraries/winpthreads/src/thread.c
> >> > +++ b/mingw-w64-libraries/winpthreads/src/thread.c
> >> > @@ -508,11 +508,25 @@ __dyn_tls_pthread (HANDLE hDllHandle, DWORD
> >> > dwReason, LPVOID lpreserved)
> >> >
> >> > /* TLS-runtime section variable. */
> >> > #ifdef _MSC_VER
> >> > -#pragma section(".CRT$XLF", shared)
> >> > +# ifdef _WIN64
> >> > +/* .CRT section is merged with .rdata on x64 so it must be constant
> >> > data. */
> >>
> >> This patch assumes that _WIN64 is enough to determine between x86 and x64,
> >> but does the distinction also hold up for ARM and ARM64? As current public
> >> MSVC releases support all these 4 architectures, I would suggest checking
> >> what the situation is wrt these on all 4 architectures before adding
> >> ifdefs like this.
> >
> > Thanks for the review!
> >
> > The page at Microsoft-specific predefined macros [1] has some
> > information about the macros.
> > - _WIN32 Defined as 1 when the compilation target is 32-bit ARM,
> > 64-bit ARM, x86, or x64. Otherwise, undefined.
> > - _WIN64 Defined as 1 when the compilation target is 64-bit ARM or
> > x64. Otherwise, undefined.
> >
> > There are also some arch-specific macros, such as _M_AMD64, _M_ARM64, etc.
> > https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170#microsoft-specific-predefined-macros
> >
> > Does that answer your question?
>
> No, that doesn't answer the question.
>
> Your patch makes the distinction that x86 should use #pragma data_seg,
> while x64 should use #pragma const_seg.
>
> Your patch also makes ARM use data_seg and ARM64 use const_seg. I don't
> see anything specifically that indicates that pointer size should affect
> which section is used for these pointers on ARM platforms.
>
> Therefore: Check which one should be used for ARM and ARM64, whichever way
> you used to check this for x86 and x64, and update your ifdef conditions
> to match this. I doubt that _WIN64 is the right distinction.
Thanks a lot for clarifying. I couldn't find any documentation about
this, apart from blog posts [1]
and related code in Chromium [2].
The first doesn't mention ARM, but both use const_seg on 64-bits
arches and data_seg on 32 bits.
[1]:
https://lallouslab.net/2017/05/30/using-cc-tls-callbacks-in-visual-studio-with-your-32-or-64bits-programs/
[2]:
https://chromium.googlesource.com/chromium/src/+/lkgr/base/win/dllmain.cc#64
I'll try to write a sensible repro case and build it on all arches. I
don't have ARM Windows machines, I hope that looking at the
cross-compiled executable sections will be enough.
_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
