perl_5.32.0~rc1-1 in experimental failed to build on m68k with a
SIGSEGV in miniperl. Could somebody please look at this and see if it's reproducible? Guess it could be either an actual source regression or
"just" a toolchain/platform problem.
On 6/17/20 10:15 PM, Niko Tyni wrote:
perl_5.32.0~rc1-1 in experimental failed to build on m68k with a
SIGSEGV in miniperl. Could somebody please look at this and see if it's
reproducible? Guess it could be either an actual source regression or
"just" a toolchain/platform problem.
Thanks for the heads-up. I'll look into it.
Reported upstream [1].Thanks for the heads-up. I'll look into it.
Here's the backtrace:
[1] https://github.com/Perl/perl5/issues/17871
Reported upstream [1].
On 6/18/20 12:10 PM, John Paul Adrian Glaubitz wrote:
Reported upstream [1].
It's an alignment issue and can be trivially fixed with this patch:
diff --git a/op.h b/op.h
index fc21f03cda..480c95245b 100644
--- a/op.h
+++ b/op.h
@@ -698,7 +698,7 @@ struct opslot {
U16 opslot_size; /* size of this slot (in pointers) */
U16 opslot_offset; /* offset from start of slab (in ptr units) */
OP opslot_op; /* the op itself */
-};
+} __attribute__ ((aligned (4)));
struct opslab {
OPSLAB * opslab_next; /* next slab */
diff --git a/op.h b/op.h
index fc21f03cda..fb9f538e23 100644
--- a/op.h
+++ b/op.h
@@ -714,6 +714,7 @@ struct opslab {
# ifdef PERL_DEBUG_READONLY_OPS
bool opslab_readonly;
# endif
+ U16 opslab_padding; /* padding to ensure proper alignment */
OPSLOT opslab_slots; /* slots begin here */
};
So release builds fail, too?
And they don't set PERL_DEBUG_READONLY_OPS?
If PERL_DEBUG_READONLY_OPS is not set, I see no reason why
the padding would be needed.
On Jun 19, 2020, at 10:02 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
Hi Adrian,
On Thu, Jun 18, 2020 at 10:46 PM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
On 6/18/20 12:10 PM, John Paul Adrian Glaubitz wrote:
Reported upstream [1].
It's an alignment issue and can be trivially fixed with this patch:
diff --git a/op.h b/op.h
index fc21f03cda..480c95245b 100644
--- a/op.h
+++ b/op.h
@@ -698,7 +698,7 @@ struct opslot {
U16 opslot_size; /* size of this slot (in pointers) */
U16 opslot_offset; /* offset from start of slab (in ptr units) */
OP opslot_op; /* the op itself */
-};
+} __attribute__ ((aligned (4)));
struct opslab {
OPSLAB * opslab_next; /* next slab */
In the mean time, you changed this to add explicit padding instead:
https://github.com/Perl/perl5/issues/17871
diff --git a/op.h b/op.h
index fc21f03cda..fb9f538e23 100644
--- a/op.h
+++ b/op.h
@@ -714,6 +714,7 @@ struct opslab {
# ifdef PERL_DEBUG_READONLY_OPS
bool opslab_readonly;
# endif
+ U16 opslab_padding; /* padding to ensure proper alignment */
OPSLOT opslab_slots; /* slots begin here */
};
I take it PERL_DEBUG_READONLY_OPS is enabled?
Hence the padding should be moved inside the #ifdef,
Furthermore, sizeof(bool) = 1, right? So you still have an implicit
hole, and it would be better to add 3 bytes of explicit padding
instead one 16-bit quantity.
On Jun 19, 2020, at 10:02 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
On Thu, Jun 18, 2020 at 10:46 PM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
On 6/18/20 12:10 PM, John Paul Adrian Glaubitz wrote:
Reported upstream [1].
It's an alignment issue and can be trivially fixed with this patch:
diff --git a/op.h b/op.h
index fc21f03cda..480c95245b 100644
--- a/op.h
+++ b/op.h
@@ -698,7 +698,7 @@ struct opslot {
U16 opslot_size; /* size of this slot (in pointers) */ >> U16 opslot_offset; /* offset from start of slab (in ptr units) */
OP opslot_op; /* the op itself */
-};
+} __attribute__ ((aligned (4)));
struct opslab {
OPSLAB * opslab_next; /* next slab */
In the mean time, you changed this to add explicit padding instead:
https://github.com/Perl/perl5/issues/17871
diff --git a/op.h b/op.h
index fc21f03cda..fb9f538e23 100644
--- a/op.h
+++ b/op.h
@@ -714,6 +714,7 @@ struct opslab {
# ifdef PERL_DEBUG_READONLY_OPS
bool opslab_readonly;
# endif
+ U16 opslab_padding; /* padding to ensure proper alignment */
OPSLOT opslab_slots; /* slots begin here */
};
I take it PERL_DEBUG_READONLY_OPS is enabled?
Hence the padding should be moved inside the #ifdef,
Furthermore, sizeof(bool) = 1, right? So you still have an implicit
hole, and it would be better to add 3 bytes of explicit padding
instead one 16-bit quantity.
No, I didn’t take that #ifdef into consideration.
And I’m confused now.
Add the three bytes how? Inside the #ifdef makes no sense as that wouldn’t fix release builds.
It might be set during stage1, but not stage2.
I will have to look at it again.
On 6/19/20 10:47 AM, John Paul Adrian Glaubitz wrote:
It might be set during stage1, but not stage2.
I will have to look at it again.
I just tried this change and it doesn't fix the problem:
--- a/op.h
+++ b/op.h
@@ -713,6 +713,8 @@ struct opslab {
units) */
# ifdef PERL_DEBUG_READONLY_OPS
bool opslab_readonly;
+ U16 _padding1; /* additional padding to ensure opslab is 32-bit aligned */
+ U8 _padding2; /* when PERL_DEBUG_READONLY_OPS is set */
# endif
OPSLOT opslab_slots; /* slots begin here */
};
Either invert the order of the two paddings, or replace them by a single one:I just tried this variant and it didn't help:
U8 _padding[3];
This works, both with PERL_DEBUG_READONLY_OPS and without:
diff --git a/op.h b/op.h
index fc21f03cda..fc7e73fef4 100644
--- a/op.h
+++ b/op.h
@@ -713,7 +713,9 @@ struct opslab {
units) */
# ifdef PERL_DEBUG_READONLY_OPS
bool opslab_readonly;
+ U8 opslab_padding1[3];
# endif
+ U16 opslab_padding2;
OPSLOT opslab_slots; /* slots begin here */
};
| Sysop: | Keyop |
|---|---|
| Location: | Huddersfield, West Yorkshire, UK |
| Users: | 293 |
| Nodes: | 16 (2 / 14) |
| Uptime: | 212:46:14 |
| Calls: | 6,619 |
| Calls today: | 1 |
| Files: | 12,168 |
| Messages: | 5,317,377 |