Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

special-static.patch can fail to build on s390x / upstream v6.8 #1381

Closed
joe-lawrence opened this issue Mar 25, 2024 · 6 comments
Closed

special-static.patch can fail to build on s390x / upstream v6.8 #1381

joe-lawrence opened this issue Mar 25, 2024 · 6 comments
Labels

Comments

@joe-lawrence
Copy link
Contributor

For some reason, gcc on s390x decides to rearrange instructions due to this patch (my own notes correlating instructions added on the right hand side) and the build fails as those changes land in the .init.text section:

Disassembly of section .init.text:

 0000000000000000 <coredump_filter_setup>:
    0:  c0 04 00 00 00 00       jgnop   0 <coredump_filter_setup>
    6:  eb ef f0 88 00 24       stmg    %r14,%r15,136(%r15)
    c:  b9 04 00 ef             lgr     %r14,%r15
@@ -122,26 +122,26 @@
 00000000000001e0 <fork_idle>:
  1e0:  c0 04 00 00 00 00       jgnop   1e0 <fork_idle>
  1e6:  eb 9f f0 60 00 24       stmg    %r9,%r15,96(%r15)
  1ec:  b9 04 00 ef             lgr     %r14,%r15
  1f0:  e3 f0 ff 40 ff 71       lay     %r15,-192(%r15)
  1f6:  e3 e0 f0 98 00 24       stg     %r14,152(%r15)
  1fc:  c4 a8 00 00 00 00       lgrl    %r10,1fc <fork_idle+0x1c>
                        1fe: R_390_GOTENT       init_struct_pid+0x2
- 202:  d7 77 f0 a8 f0 a8       xc      168(120,%r15),168(%r15)            A
- 208:  c0 10 00 00 00 00       larl    %r1,208 <fork_idle+0x28>           B
-                       20a: R_390_PLT32DBL     idle_dummy+0x2
+ 202:  c0 10 00 00 00 00       larl    %r1,202 <fork_idle+0x22>           B
+                       204: R_390_PLT32DBL     idle_dummy+0x2
+ 208:  d7 77 f0 a8 f0 a8       xc      168(120,%r15),168(%r15)            A
  20e:  b9 04 00 92             lgr     %r9,%r2
- 212:  e3 10 f1 00 00 24       stg     %r1,256(%r15)                      C
- 218:  92 80 f0 cc             mvi     204(%r15),128                      D
- 21c:  e5 48 f0 a0 01 00       mvghi   160(%r15),256                      E
- 222:  e5 4c f0 fc 00 01       mvhi    252(%r15),1                        F
- 228:  b9 04 00 2a             lgr     %r2,%r10                           G
- 22c:  41 50 f0 a0             la      %r5,160(%r15)                      H
+ 212:  41 50 f0 a0             la      %r5,160(%r15)                      H
+ 216:  e3 10 f1 00 00 24       stg     %r1,256(%r15)                      C
+ 21c:  92 80 f0 cc             mvi     204(%r15),128                      D
+ 220:  e5 48 f0 a0 01 00       mvghi   160(%r15),256                      E
+ 226:  e5 4c f0 fc 00 01       mvhi    252(%r15),1                        F
+ 22c:  b9 04 00 2a             lgr     %r2,%r10                           G
  230:  a7 49 00 00             lghi    %r4,0
  234:  a7 39 00 00             lghi    %r3,0
  238:  c0 e5 00 00 00 00       brasl   %r14,238 <fork_idle+0x58>
                        23a: R_390_PLT32DBL     copy_process+0x2
  23e:  a7 19 f0 00             lghi    %r1,-4096
  242:  b9 04 00 b2             lgr     %r11,%r2
  246:  ec 21 00 2d 20 65       clgrjh  %r2,%r1,2a0 <fork_idle+0xc0>
  24c:  e3 a0 27 f8 00 24       stg     %r10,2040(%r2)

The fork_idle() function was annotated as __init way back in v5.14 torvalds/linux@f1a0a376ca0c ("sched/core: Initialize the idle task with preemption disabled"), so I'm surprised this is only failing now.

@jpoimboe : two questions about this test:

  1. special-static.patch was introduced to verify that kpatch-build could handle __warned and __key so-called special static local variables. Do these special symbols need to exist in kpatch-modified functions, or merely somewhere in kpatch-modified object files?
  2. Could this be turned into an unit test? The patch doesn't do much, and nothing I see specifically regarding the special symbols.
@jpoimboe
Copy link
Member

Hm, that's odd. I haven't seen anything like that before. Maybe changes to non-bundled sections could be downgraded to warnings. Or if this seems like a fluke we could do KPATCH_IGNORE_SECTION() for that patch.

  • special-static.patch was introduced to verify that kpatch-build could handle __warned and __key so-called special static local variables. Do these special symbols need to exist in kpatch-modified functions, or merely somewhere in kpatch-modified object files?

I'm not 100% sure, but I think the current test triggers a change in a function which actually uses such variables, so let's stick with that :-)

  • Could this be turned into an unit test? The patch doesn't do much, and nothing I see specifically regarding the special symbols.

Yeah, I think so.

@joe-lawrence
Copy link
Contributor Author

Here are the object files for special-static.patch as built on RHEL-9.3:
special-static-objs.tar.gz

It looks like copy_signal() only references a few __key variables, though the greater fork.o file a few others (like __already_done, _entry, and __func__). If that's good enough, then I can push the stripped objects into the unit-test repo and deprecate the special-static.patch.

Copy link

github-actions bot commented May 3, 2024

This issue has been open for 30 days with no activity and no assignee. It will be closed in 7 days unless a comment is added.

@github-actions github-actions bot added the stale label May 3, 2024
Copy link

This issue was closed because it was inactive for 7 days after being marked stale.

Copy link

This issue has been open for 30 days with no activity and no assignee. It will be closed in 7 days unless a comment is added.

@github-actions github-actions bot added the stale label Jun 10, 2024
Copy link

This issue was closed because it was inactive for 7 days after being marked stale.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants