[PATCH] msvcp90: Fix vtable alignment on macOS.
On macOS, the .align <n> directive aligns to 2^n, not just n. Signed-off-by: Ken Thomases <ken(a)codeweavers.com> --- dlls/msvcp90/cxx.h | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/dlls/msvcp90/cxx.h b/dlls/msvcp90/cxx.h index 917d814..86295cd 100644 --- a/dlls/msvcp90/cxx.h +++ b/dlls/msvcp90/cxx.h @@ -43,9 +43,15 @@ #define VTABLE_ADD_FUNC(name) "\t.quad " THISCALL_NAME(name) "\n" +#ifdef __APPLE__ +#define __ASM_VTABLE_ALIGN ".align 3" +#else +#define __ASM_VTABLE_ALIGN ".align 8" +#endif + #define __ASM_VTABLE(name,funcs) \ __asm__(".data\n" \ - "\t.align 8\n" \ + "\t" __ASM_VTABLE_ALIGN "\n" \ "\t.quad " __ASM_NAME(#name "_rtti") "\n" \ "\t.globl " __ASM_NAME("MSVCP_" #name "_vtable") "\n" \ __ASM_NAME("MSVCP_" #name "_vtable") ":\n" \ @@ -55,9 +61,15 @@ #define VTABLE_ADD_FUNC(name) "\t.long " THISCALL_NAME(name) "\n" +#ifdef __APPLE__ +#define __ASM_VTABLE_ALIGN ".align 2" +#else +#define __ASM_VTABLE_ALIGN ".align 4" +#endif + #define __ASM_VTABLE(name,funcs) \ __asm__(".data\n" \ - "\t.align 4\n" \ + "\t" __ASM_VTABLE_ALIGN "\n" \ "\t.long " __ASM_NAME(#name "_rtti") "\n" \ "\t.globl " __ASM_NAME("MSVCP_" #name "_vtable") "\n" \ __ASM_NAME("MSVCP_" #name "_vtable") ":\n" \ -- 2.10.2
March 6, 2019 7:01 PM, "Ken Thomases" <ken(a)codeweavers.com> wrote:
On macOS, the .align <n> directive aligns to 2^n, not just n.
I wonder if we should explicitly use '.balign 8', since that always means "align to 8 bytes." Unless, of course, there are platforms that don't support it.
Signed-off-by: Ken Thomases <ken(a)codeweavers.com> --- dlls/msvcp90/cxx.h | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/dlls/msvcp90/cxx.h b/dlls/msvcp90/cxx.h index 917d814..86295cd 100644 --- a/dlls/msvcp90/cxx.h +++ b/dlls/msvcp90/cxx.h @@ -43,9 +43,15 @@
#define VTABLE_ADD_FUNC(name) "\t.quad " THISCALL_NAME(name) "\n"
+#ifdef __APPLE__ +#define __ASM_VTABLE_ALIGN ".align 3" +#else +#define __ASM_VTABLE_ALIGN ".align 8" +#endif + #define __ASM_VTABLE(name,funcs) \ __asm__(".data\n" \ - "\t.align 8\n" \ + "\t" __ASM_VTABLE_ALIGN "\n" \ "\t.quad " __ASM_NAME(#name "_rtti") "\n" \ "\t.globl " __ASM_NAME("MSVCP_" #name "_vtable") "\n" \ __ASM_NAME("MSVCP_" #name "_vtable") ":\n" \ @@ -55,9 +61,15 @@
#define VTABLE_ADD_FUNC(name) "\t.long " THISCALL_NAME(name) "\n"
+#ifdef __APPLE__ +#define __ASM_VTABLE_ALIGN ".align 2" +#else +#define __ASM_VTABLE_ALIGN ".align 4" +#endif + #define __ASM_VTABLE(name,funcs) \ __asm__(".data\n" \ - "\t.align 4\n" \ + "\t" __ASM_VTABLE_ALIGN "\n" \ "\t.long " __ASM_NAME(#name "_rtti") "\n" \ "\t.globl " __ASM_NAME("MSVCP_" #name "_vtable") "\n" \ __ASM_NAME("MSVCP_" #name "_vtable") ":\n" \ -- 2.10.2
Chip
On Mar 6, 2019, at 9:28 PM, Chip Davis <cdavis(a)codeweavers.com> wrote:
March 6, 2019 7:01 PM, "Ken Thomases" <ken(a)codeweavers.com> wrote:
On macOS, the .align <n> directive aligns to 2^n, not just n.
I wonder if we should explicitly use '.balign 8', since that always means "align to 8 bytes." Unless, of course, there are platforms that don't support it.
This is a general issue that winebuild also explicitly contends with, and it isn't using .balign (nor .p2align). See tools/winebuild/utils.c:get_alignment(). So, I assume .balign isn't an acceptable solution. I'd be happy to be told otherwise, though. -Ken
Ken Thomases <ken(a)codeweavers.com> writes:
On Mar 6, 2019, at 9:28 PM, Chip Davis <cdavis(a)codeweavers.com> wrote:
March 6, 2019 7:01 PM, "Ken Thomases" <ken(a)codeweavers.com> wrote:
On macOS, the .align <n> directive aligns to 2^n, not just n.
I wonder if we should explicitly use '.balign 8', since that always means "align to 8 bytes." Unless, of course, there are platforms that don't support it.
This is a general issue that winebuild also explicitly contends with, and it isn't using .balign (nor .p2align). See tools/winebuild/utils.c:get_alignment(). So, I assume .balign isn't an acceptable solution. I'd be happy to be told otherwise, though.
It's not supported on Solaris, which was the reason for the winebuild code. I'm not sure if anybody still cares about Solaris nowadays... -- Alexandre Julliard julliard(a)winehq.org
Hi Ken, On 3/7/19 9:47 AM, Alexandre Julliard wrote:
Ken Thomases <ken(a)codeweavers.com> writes:
On Mar 6, 2019, at 9:28 PM, Chip Davis <cdavis(a)codeweavers.com> wrote:
March 6, 2019 7:01 PM, "Ken Thomases" <ken(a)codeweavers.com> wrote:
On macOS, the .align <n> directive aligns to 2^n, not just n.
I wonder if we should explicitly use '.balign 8', since that always means "align to 8 bytes." Unless, of course, there are platforms that don't support it.
This is a general issue that winebuild also explicitly contends with, and it isn't using .balign (nor .p2align). See tools/winebuild/utils.c:get_alignment(). So, I assume .balign isn't an acceptable solution. I'd be happy to be told otherwise, though.
It's not supported on Solaris, which was the reason for the winebuild code. I'm not sure if anybody still cares about Solaris nowadays... Shouldn't it be changed on ARM, ARM64 and PowerPC as well? Maybe it's better to use .balign for all platforms except Solaris (or ignore Solaris until someone reports bug for it?).
Thanks, Piotr
On 3/7/19 9:47 AM, Alexandre Julliard wrote:
Ken Thomases <ken(a)codeweavers.com> writes:
On Mar 6, 2019, at 9:28 PM, Chip Davis <cdavis(a)codeweavers.com> wrote:
March 6, 2019 7:01 PM, "Ken Thomases" <ken(a)codeweavers.com> wrote:
On macOS, the .align <n> directive aligns to 2^n, not just n.
I wonder if we should explicitly use '.balign 8', since that always means "align to 8 bytes." Unless, of course, there are platforms that don't support it.
This is a general issue that winebuild also explicitly contends with, and it isn't using .balign (nor .p2align). See tools/winebuild/utils.c:get_alignment(). So, I assume .balign isn't an acceptable solution. I'd be happy to be told otherwise, though.
It's not supported on Solaris, which was the reason for the winebuild code. I'm not sure if anybody still cares about Solaris nowadays.. The last commit mentioning Solaris is from 2016 Francois. So if he doesn't cares anymore about it I doubt anybody else does.
bye michael
On Thu, Mar 7, 2019, 09:48 Alexandre Julliard <julliard(a)winehq.org> wrote:
Ken Thomases <ken(a)codeweavers.com> writes:
On Mar 6, 2019, at 9:28 PM, Chip Davis <cdavis(a)codeweavers.com> wrote:
March 6, 2019 7:01 PM, "Ken Thomases" <ken(a)codeweavers.com> wrote:
On macOS, the .align <n> directive aligns to 2^n, not just n.
I wonder if we should explicitly use '.balign 8', since that always
means "align to 8 bytes." Unless, of course, there are platforms that don't support it.
This is a general issue that winebuild also explicitly contends with, and it isn't using .balign (nor .p2align). See tools/winebuild/utils.c:get_alignment(). So, I assume .balign isn't an acceptable solution. I'd be happy to be told otherwise, though.
It's not supported on Solaris, which was the reason for the winebuild code. I'm not sure if anybody still cares about Solaris nowadays...
-- Alexandre Julliard julliard(a)winehq.org
Does this also mean we can kill backticks in shell scripts? ;)
Austin English <austinenglish(a)gmail.com> writes:
On Thu, Mar 7, 2019, 09:48 Alexandre Julliard <julliard(a)winehq.org> wrote:
Ken Thomases <ken(a)codeweavers.com> writes:
On Mar 6, 2019, at 9:28 PM, Chip Davis <cdavis(a)codeweavers.com> wrote:
March 6, 2019 7:01 PM, "Ken Thomases" <ken(a)codeweavers.com> wrote:
On macOS, the .align <n> directive aligns to 2^n, not just n.
I wonder if we should explicitly use '.balign 8', since that always means "align to 8 bytes." Unless, of course, there are platforms that don't support it.
This is a general issue that winebuild also explicitly contends with, and it isn't using .balign (nor .p2align). See tools/winebuild/utils.c:get_alignment(). So, I assume .balign isn't an acceptable solution. I'd be happy to be told otherwise, though.
It's not supported on Solaris, which was the reason for the winebuild code. I'm not sure if anybody still cares about Solaris nowadays...
-- Alexandre Julliard julliard(a)winehq.org
Does this also mean we can kill backticks in shell scripts? ;)
There's no reason to change existing scripts, but we can stop using backticks in new code and see if anybody complains. -- Alexandre Julliard julliard(a)winehq.org
participants (6)
-
Alexandre Julliard -
Austin English -
Chip Davis -
Ken Thomases -
Michael Stefaniuc -
Piotr Caban