Bug 1214528 - [Build 20230822] icewm segfaults during installation
Summary: [Build 20230822] icewm segfaults during installation
Status: RESOLVED FIXED
Alias: None
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: Basesystem (show other bugs)
Version: Current
Hardware: 32bit Other
: P5 - None : Normal (vote)
Target Milestone: ---
Assignee: Simon Lees
QA Contact: E-mail List
URL: https://openqa.opensuse.org/tests/352...
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-08-23 12:06 UTC by Dominique Leuenberger
Modified: 2023-09-06 07:35 UTC (History)
1 user (show)

See Also:
Found By: openQA
Services Priority:
Business Priority:
Blocker: Yes
Marketing QA Status: ---
IT Deployment: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dominique Leuenberger 2023-08-23 12:06:09 UTC
## Observation

from the serial log of the test:
begin 644 icewm.core.pid_3737.sig_11.time_1692714349

there is a 'coredump' submitted onto the serial log, but I doubt this is very useful


openQA test in scenario opensuse-Tumbleweed-LegacyX86-DVD-i586-gnome@32bit fails in
[online_repos](https://openqa.opensuse.org/tests/3523214/modules/online_repos/steps/3)

## Test suite description
Revert schedule to working settings - dleuenberger, 20211117


## Reproducible

Fails since (at least) Build [20230821](https://openqa.opensuse.org/tests/3521433)


## Expected result

Last good: [20230819](https://openqa.opensuse.org/tests/3518371) (or more recent)


## Further details

Always latest result in this scenario: [latest](https://openqa.opensuse.org/tests/latest?arch=i586&distri=opensuse&flavor=LegacyX86-DVD&machine=32bit&test=gnome&version=Tumbleweed)
Comment 1 Dominique Leuenberger 2023-08-31 09:32:08 UTC
Fabian traced this down to imlib2 (which was updated to 1.12.0 in the snapshot that started showing this issue.

IMLIB2_ASM_OFF=1 seems to work around the issue - so giving hints that this indeed needs to be fixed in imlib2

Assigning to the imlib2 maintainer
Comment 2 Fabian Vogt 2023-08-31 09:54:09 UTC
Here's the backtrace:

#0  __imlib_Scale_mmx_AARGBA () at asm_scale.S:610
#1  0xb7ee3f4a in __imlib_ScaleAARGBA (dx=<optimized out>, dy=<optimized out>, sow=<optimized out>, dow=<optimized out>, dh=<optimized out>, dw=<optimized out>, dyy=<optimized out>, dxx=<optimized out>, dest=<optimized out>, 
    srce=<optimized out>, isi=<optimized out>) at /usr/src/debug/imlib2-1.12.0/src/lib/scale.c:292
#2  __imlib_Scale.constprop.0 (isi=isi@entry=0x1c2ce90, aa=aa@entry=true, alpha=<optimized out>, srce=0x1c2d4d0, dest=0x1c3d4e0, dxx=0, dyy=0, dw=16, dh=16, dow=16, sow=128, dy=0, dx=0) at /usr/src/debug/imlib2-1.12.0/src/lib/scale.c:1060
#3  0xb7ec0e85 in __imlib_BlendImageToImage (im_src=0x1c2cdd0, im_dst=0x1c2ce30, aa=<optimized out>, blend=<optimized out>, merge_alpha=<optimized out>, ssx=0, ssy=0, ssw=128, ssh=128, ddx=0, ddy=0, ddw=16, ddh=16, cm=0x0, op=0, clx=0, 
    cly=0, clw=0, clh=0) at /usr/src/debug/imlib2-1.12.0/src/lib/blend.c:1947
#4  0xb7eb0f91 in imlib_create_cropped_scaled_image (src_x=0, src_y=0, src_width=128, src_height=128, dst_width=16, dst_height=16) at /usr/src/debug/imlib2-1.12.0/src/lib/api.c:1076

I guess some of the refactoring upstream broke assumptions in the MMX (!) code.

FWICT the official repo is at https://git.enlightenment.org/old/legacy-imlib2/. Having both "old" and "legacy" in there doesn't instill confidence.
Comment 3 Fabian Vogt 2023-08-31 13:04:46 UTC
(In reply to Fabian Vogt from comment #2)
> I guess some of the refactoring upstream broke assumptions in the MMX (!)
> code.

Yep.

commit ee61df687706d4907fb9dcc8f402ee8bdc321db2
Author: Kim Woelders <kim@woelders.dk>
Date:   Tue May 9 15:36:25 2023 +0200

    scaling: Change ypoints[] from pointers to indices

diff --git a/src/lib/scale.c b/src/lib/scale.c
index 9307553..a79c3fc 100644
--- a/src/lib/scale.c
+++ b/src/lib/scale.c
@@ -11,8 +11,9 @@
 /*\ NB: If you change this, don't forget asm_scale.S \*/
 struct _imlib_scale_info {
    int                *xpoints;
-   uint32_t          **ypoints;
-   int                *xapoints, *yapoints;
+   int                *ypoints;
+   int                *xapoints;
+   int                *yapoints;
    int                 xup_yup;
    uint32_t           *pix_assert;
 };

You can guess what was forgotten here.
Comment 4 Simon Lees 2023-09-01 02:10:25 UTC
(In reply to Fabian Vogt from comment #2)
>
> FWICT the official repo is at
> https://git.enlightenment.org/old/legacy-imlib2/. Having both "old" and
> "legacy" in there doesn't instill confidence.

This is not actually a huge concern, Its here because the Enlightenment project stopped using imlib2 15-20 years ago (There replacement is now part of libefl/libevas). Since then the project has been actively maintained by other people (hence getting a 1.2.0 release).
Comment 5 Simon Lees 2023-09-01 02:31:04 UTC
(In reply to Fabian Vogt from comment #3)
> (In reply to Fabian Vogt from comment #2)
> > I guess some of the refactoring upstream broke assumptions in the MMX (!)
> > code.
> 
> Yep.
> 
> commit ee61df687706d4907fb9dcc8f402ee8bdc321db2
> Author: Kim Woelders <kim@woelders.dk>
> Date:   Tue May 9 15:36:25 2023 +0200
> 
>     scaling: Change ypoints[] from pointers to indices
> 
> diff --git a/src/lib/scale.c b/src/lib/scale.c
> index 9307553..a79c3fc 100644
> --- a/src/lib/scale.c
> +++ b/src/lib/scale.c
> @@ -11,8 +11,9 @@
>  /*\ NB: If you change this, don't forget asm_scale.S \*/
>  struct _imlib_scale_info {
>     int                *xpoints;
> -   uint32_t          **ypoints;
> -   int                *xapoints, *yapoints;
> +   int                *ypoints;
> +   int                *xapoints;
> +   int                *yapoints;
>     int                 xup_yup;
>     uint32_t           *pix_assert;
>  };
> 
> You can guess what was forgotten here.

I'll presume asm_scale.S given it hasn't changed. I'm not competent enough with intel assembly to come up with a patch, so i'll accept your disabling MXX as a work around for now and I'll raise this with upstream.
Comment 6 Simon Lees 2023-09-06 07:35:04 UTC
Upstream has decided to disable the scaling code in Assembly (x86_64 is done by the compiler anyway). As such https://build.opensuse.org/request/show/1108295 will be a sufficient fix (the upstream fix just sets that option to false regardless of the flag).