1*2ab23a58SSong Liu.. SPDX-License-Identifier: GPL-2.0 2*2ab23a58SSong Liu 3*2ab23a58SSong Liu===================================== 4*2ab23a58SSong LiuWriting a test for objtool's klp code 5*2ab23a58SSong Liu===================================== 6*2ab23a58SSong Liu 7*2ab23a58SSong LiuInstructions for adding a test to tools/objtool/tests/. Read 8*2ab23a58SSong Liuklp-test-design.txt first if you need to know how the harness works; this 9*2ab23a58SSong Liudocument is the procedure and the rules. 10*2ab23a58SSong Liu 11*2ab23a58SSong LiuTwo kinds of request bring you here: 12*2ab23a58SSong Liu 13*2ab23a58SSong Liu* *"write a test for commit <sha>"* -- a fix went in without one. 14*2ab23a58SSong Liu* *"port the test at <location>, written against another harness"* -- a case 15*2ab23a58SSong Liu exists elsewhere and should live in tree. 16*2ab23a58SSong Liu 17*2ab23a58SSong LiuBoth follow the same procedure. 18*2ab23a58SSong Liu 19*2ab23a58SSong Liu 20*2ab23a58SSong LiuThe one rule that matters 21*2ab23a58SSong Liu========================= 22*2ab23a58SSong Liu 23*2ab23a58SSong Liu**A test is not finished until you have watched it fail.** 24*2ab23a58SSong Liu 25*2ab23a58SSong LiuBreak the thing it guards -- revert the fix, or sabotage the exact line -- and 26*2ab23a58SSong Liuconfirm the test fails. Then restore and confirm it passes. A test that has 27*2ab23a58SSong Liunever failed is not known to test anything, and this suite has produced 28*2ab23a58SSong Liuseveral that passed against deliberately broken code: 29*2ab23a58SSong Liu 30*2ab23a58SSong Liu* an alternatives fixture whose empty entry pointed at its own end label rather 31*2ab23a58SSong Liu than the neighbour's replacement, so the bug it guarded made no difference; 32*2ab23a58SSong Liu* a sympos fixture where symbol-table order and address order agreed, so 33*2ab23a58SSong Liu counting and reading the linked image gave the same answer; 34*2ab23a58SSong Liu* a string fixture using a named ``char[]``, which never reached the 35*2ab23a58SSong Liu contents-hashing path because that keys on ``SHF_STRINGS``; 36*2ab23a58SSong Liu* a static array the compiler proved constant, folded to zero, and emitted no 37*2ab23a58SSong Liu relocation for -- so the two builds were byte-identical. 38*2ab23a58SSong Liu 39*2ab23a58SSong LiuEvery one looked correct. Say in the commit message how you verified, and if 40*2ab23a58SSong Liuyou could not isolate the behaviour to a single line, **say that too** rather 41*2ab23a58SSong Liuthan implying otherwise. 42*2ab23a58SSong Liu 43*2ab23a58SSong Liu 44*2ab23a58SSong LiuProcedure 45*2ab23a58SSong Liu========= 46*2ab23a58SSong Liu 47*2ab23a58SSong Liu1. **Read the fix.** What input reaches the broken line? What is observable 48*2ab23a58SSong Liu in the output object when it misbehaves -- a missing section, a relocation 49*2ab23a58SSong Liu naming the wrong symbol, an unchanged checksum, a rejected build? If 50*2ab23a58SSong Liu nothing is observable, stop and say so; see `When to give up`_. 51*2ab23a58SSong Liu 52*2ab23a58SSong Liu2. **Decide where it lives.** ``generic/`` unless the fixture needs 53*2ab23a58SSong Liu architecture-specific assembly or the behaviour is architecture-specific, 54*2ab23a58SSong Liu in which case ``x86/`` (or a new directory named for the architecture). 55*2ab23a58SSong Liu 56*2ab23a58SSong Liu3. **Write the fixture** in the same directory's ``fixtures/``. Reuse an 57*2ab23a58SSong Liu existing one if it already produces the shape; add a ``-D`` knob rather 58*2ab23a58SSong Liu than copying a fixture to change one line. 59*2ab23a58SSong Liu 60*2ab23a58SSong Liu4. **Write the test.** Assert the *premise* before the result -- see 61*2ab23a58SSong Liu `State the premise`_. 62*2ab23a58SSong Liu 63*2ab23a58SSong Liu5. **Verify by breaking the code.** Then restore. 64*2ab23a58SSong Liu 65*2ab23a58SSong Liu6. **Run the whole suite under both compilers**:: 66*2ab23a58SSong Liu 67*2ab23a58SSong Liu make -C tools/objtool tests 68*2ab23a58SSong Liu CC=clang make -C tools/objtool tests 69*2ab23a58SSong Liu 70*2ab23a58SSong Liu7. **Commit** the test and its fixture together, alone. One test per commit. 71*2ab23a58SSong Liu 72*2ab23a58SSong Liu 73*2ab23a58SSong LiuWriting the fixture 74*2ab23a58SSong Liu=================== 75*2ab23a58SSong Liu 76*2ab23a58SSong LiuFixtures are freestanding C. No kernel headers -- write out the kernel 77*2ab23a58SSong Liustructure by hand if you need one, as the existing special-section fixtures do. 78*2ab23a58SSong Liu 79*2ab23a58SSong LiuEvery fixture needs a ``.modinfo`` name, because klp diff reads the object's 80*2ab23a58SSong Liumodule name from it:: 81*2ab23a58SSong Liu 82*2ab23a58SSong Liu static const char __modinfo[] 83*2ab23a58SSong Liu __attribute__((section(".modinfo"), used, aligned(1))) = "\0name=vmlinux"; 84*2ab23a58SSong Liu 85*2ab23a58SSong LiuUse ``MODNAME`` if the test needs to vary it. Note that most fixtures hardcode 86*2ab23a58SSong Liu``vmlinux``: passing ``-DMODNAME`` to one that does silently does nothing and 87*2ab23a58SSong Liuthe test quietly becomes a vmlinux test. 88*2ab23a58SSong Liu 89*2ab23a58SSong LiuThe patched build is selected with ``-DPATCHED``. For a fixture with several 90*2ab23a58SSong Liuvariants, gate each on both, so the original is always the baseline:: 91*2ab23a58SSong Liu 92*2ab23a58SSong Liu #if defined(PATCHED) && defined(WHICH_CALL) 93*2ab23a58SSong Liu r = callee_b(x); 94*2ab23a58SSong Liu #else 95*2ab23a58SSong Liu r = callee_a(x); 96*2ab23a58SSong Liu #endif 97*2ab23a58SSong Liu 98*2ab23a58SSong Liuand select one per build: ``build_pair foo.c -DWHICH_CALL``. Without the 99*2ab23a58SSong Liu``defined(PATCHED)`` the flag applies to *both* builds and nothing differs. 100*2ab23a58SSong Liu 101*2ab23a58SSong LiuTraps that have bitten before 102*2ab23a58SSong Liu----------------------------- 103*2ab23a58SSong Liu 104*2ab23a58SSong Liu* **The compiler optimises your fixture away.** A static never written is 105*2ab23a58SSong Liu proved constant, its reads folded, and no relocation emitted. Add a writer 106*2ab23a58SSong Liu the compiler cannot see through. 107*2ab23a58SSong Liu* **String literals versus named arrays.** The contents-hashing path keys on 108*2ab23a58SSong Liu ``SHF_STRINGS``, which the compiler sets on the mergeable section a *literal* 109*2ab23a58SSong Liu lands in, not on a ``char[]`` given a section of its own. 110*2ab23a58SSong Liu* **Per-function sections hide movement.** With the default 111*2ab23a58SSong Liu ``-ffunction-sections`` every function sits at offset 0 of its own section, 112*2ab23a58SSong Liu so nothing ever moves. A test about position needs 113*2ab23a58SSong Liu ``build_pair foo.c -fno-function-sections``. 114*2ab23a58SSong Liu* **Special sections need boundaries.** Either an entsize on the section or an 115*2ab23a58SSong Liu ``ANNOTATE_DATA_SPECIAL`` annotation, or klp diff reports "missing special 116*2ab23a58SSong Liu section entsize or annotations". Their targets need real (global) symbols, 117*2ab23a58SSong Liu or it reports "failed to convert reloc sym". 118*2ab23a58SSong Liu* **Prefer letting objtool generate what objtool generates.** 119*2ab23a58SSong Liu ``.static_call_sites``, ``.mcount_loc``, ``.ibt_endbr_seal`` and ORC come 120*2ab23a58SSong Liu from its check pass. Call ``run_objtool_check --mcount`` and let it build 121*2ab23a58SSong Liu them; a hand-written copy tests your reading of the format, not the format. 122*2ab23a58SSong Liu 123*2ab23a58SSong Liu Write one by hand only when the test needs a shape objtool will not produce, 124*2ab23a58SSong Liu and say so in the fixture. generic/fixtures/static_call.c does: objtool 125*2ab23a58SSong Liu emits the site but not the ``ANNOTATE_DATA_SPECIAL`` that describes its 126*2ab23a58SSong Liu boundaries -- those come from the kernel's macros -- so a fixture which has 127*2ab23a58SSong Liu to vary whether the annotation is there writes both itself. 128*2ab23a58SSong Liu 129*2ab23a58SSong Liu 130*2ab23a58SSong LiuWriting the test 131*2ab23a58SSong Liu================ 132*2ab23a58SSong Liu 133*2ab23a58SSong LiuStart from the shortest existing test, generic/test-basic.sh. 134*2ab23a58SSong Liu 135*2ab23a58SSong LiuState the premise 136*2ab23a58SSong Liu----------------- 137*2ab23a58SSong Liu 138*2ab23a58SSong LiuA test which asserts only on the output passes when the compiler never emitted 139*2ab23a58SSong Liuthe construct in the first place, and reads as coverage it does not have. Say 140*2ab23a58SSong Liuwhat the input must contain:: 141*2ab23a58SSong Liu 142*2ab23a58SSong Liu assert_input_section __jump_table # the fixture must produce it -> fail 143*2ab23a58SSong Liu require_input_section .kcfi_traps # this compiler may not -> skip 144*2ab23a58SSong Liu 145*2ab23a58SSong LiuPrefer ``assert_*``. Reach for ``require_*`` only where absence genuinely 146*2ab23a58SSong Liudepends on compiler version or flags, and follow it with something 147*2ab23a58SSong Liuunconditional so the test can never be entirely vacuous. 148*2ab23a58SSong Liu 149*2ab23a58SSong LiuWhere a test would otherwise duplicate a sibling, assert what makes it 150*2ab23a58SSong Liudifferent. ``test-jump-label-module-static-key`` checks that the key really is 151*2ab23a58SSong Liureached through its section symbol -- without that it is a second copy of 152*2ab23a58SSong Liu``test-jump-label-module-key``. 153*2ab23a58SSong Liu 154*2ab23a58SSong LiuAssert both directions 155*2ab23a58SSong Liu---------------------- 156*2ab23a58SSong Liu 157*2ab23a58SSong LiuCheck that the right thing happened *and* that the wrong thing did not. A klp 158*2ab23a58SSong Liudiff which clones everything is as wrong as one which clones nothing:: 159*2ab23a58SSong Liu 160*2ab23a58SSong Liu assert_patched changed 161*2ab23a58SSong Liu assert_not_patched untouched 162*2ab23a58SSong Liu 163*2ab23a58SSong LiuSkips 164*2ab23a58SSong Liu----- 165*2ab23a58SSong Liu 166*2ab23a58SSong Liu* ``gcc_only``/``clang_only`` -- a settled fact about the compiler. Declared, 167*2ab23a58SSong Liu so it reads as expected forever. 168*2ab23a58SSong Liu* ``probe_skip`` -- this toolchain did not produce the construct. Include what 169*2ab23a58SSong Liu to do about it if there is anything:: 170*2ab23a58SSong Liu 171*2ab23a58SSong Liu probe_skip "no matching clang/lld pair for a ThinLTO link; set THIN_CC and THIN_LD to one" 172*2ab23a58SSong Liu 173*2ab23a58SSong Liu* A bare ``skip`` is **counted as a failure**. Never use it. 174*2ab23a58SSong Liu 175*2ab23a58SSong LiuStanding in for a kernel configuration 176*2ab23a58SSong Liu-------------------------------------- 177*2ab23a58SSong Liu 178*2ab23a58SSong Liu``FIXTURE_CFLAGS`` is what a fixture is built with. Since objtool reads no 179*2ab23a58SSong Liu``.config``, changing these flags is how a test covers a configuration without 180*2ab23a58SSong Liubuilding a kernel. Two ways: 181*2ab23a58SSong Liu 182*2ab23a58SSong Liu* trailing arguments to ``build_pair``/``build_one``, which win, and cover 183*2ab23a58SSong Liu anything expressible as a negation:: 184*2ab23a58SSong Liu 185*2ab23a58SSong Liu build_pair foo.c -fno-function-sections 186*2ab23a58SSong Liu 187*2ab23a58SSong Liu* otherwise assign ``FIXTURE_CFLAGS`` before building. 188*2ab23a58SSong Liu 189*2ab23a58SSong LiuEither way **say in a comment which kernel configuration the change stands in 190*2ab23a58SSong Liufor**. A flag with no stated motive is indistinguishable from a mistake. 191*2ab23a58SSong Liu 192*2ab23a58SSong Liu 193*2ab23a58SSong LiuWhat the test's comment must say 194*2ab23a58SSong Liu================================ 195*2ab23a58SSong Liu 196*2ab23a58SSong LiuThe comment at the top is the test's justification. It should let a reader 197*2ab23a58SSong Liudecide, without archaeology, whether a skip or a failure matters. Include: 198*2ab23a58SSong Liu 199*2ab23a58SSong Liu* **what breaks** in the running kernel if the behaviour regresses -- not the 200*2ab23a58SSong Liu mechanism, the consequence; 201*2ab23a58SSong Liu* **why it is not caught otherwise**, which is usually "nothing fails at build 202*2ab23a58SSong Liu time"; 203*2ab23a58SSong Liu* **the fix commit** it guards, if there is one; 204*2ab23a58SSong Liu* **anything load-bearing about the fixture** that is not obvious, especially 205*2ab23a58SSong Liu anything you got wrong first. 206*2ab23a58SSong Liu 207*2ab23a58SSong LiuThat last point is the one people skip. If the fixture has to be built without 208*2ab23a58SSong Liuper-function sections, or the static must not be named ``__warned``, or the key 209*2ab23a58SSong Liumust be file-local -- write it down, or the next person will simplify it away. 210*2ab23a58SSong Liu 211*2ab23a58SSong Liu 212*2ab23a58SSong LiuPorting a test from another harness 213*2ab23a58SSong Liu=================================== 214*2ab23a58SSong Liu 215*2ab23a58SSong LiuRead the original's *case*, not its code. The other harness probably builds a 216*2ab23a58SSong Liureal kernel module; here you write freestanding C. A transliteration will 217*2ab23a58SSong Liuusually test something else. 218*2ab23a58SSong Liu 219*2ab23a58SSong Liu* Work out which objtool behaviour the case exercises, then produce that shape 220*2ab23a58SSong Liu the cheapest way here. 221*2ab23a58SSong Liu* Verify by breaking the code, exactly as for a new test -- a port is not 222*2ab23a58SSong Liu correct because the original was. 223*2ab23a58SSong Liu* If the original names a fix commit, cite it. 224*2ab23a58SSong Liu* Credit the source in the commit message with the trailers the original 225*2ab23a58SSong Liu carried, followed by your own. 226*2ab23a58SSong Liu 227*2ab23a58SSong LiuSometimes the port shows the case is already covered, and sometimes it shows 228*2ab23a58SSong Liuthe case cannot be reproduced here. Both are results; report them rather than 229*2ab23a58SSong Liucommitting something that passes vacuously. 230*2ab23a58SSong Liu 231*2ab23a58SSong Liu 232*2ab23a58SSong LiuWhen to give up 233*2ab23a58SSong Liu=============== 234*2ab23a58SSong Liu 235*2ab23a58SSong LiuSome behaviour cannot be reached from a compiled fixture. Say so, with what 236*2ab23a58SSong Liuyou tried, instead of committing a test that passes either way. Examples that 237*2ab23a58SSong Liuwere genuinely abandoned: 238*2ab23a58SSong Liu 239*2ab23a58SSong Liu* a memory leak -- needs valgrind, not an assertion on ELF; 240*2ab23a58SSong Liu* ``mkstemp`` with long paths, and other I/O edge cases; 241*2ab23a58SSong Liu* a NULL dereference reachable only through a debug path; 242*2ab23a58SSong Liu* changes made redundant by a fallback: removing the code changes no output 243*2ab23a58SSong Liu because something else already handles the case; 244*2ab23a58SSong Liu* a fix whose code has since been rewritten, so there is nothing left to 245*2ab23a58SSong Liu revert. 246*2ab23a58SSong Liu 247*2ab23a58SSong LiuAlso stop when the behaviour depends on something outside the fixture's 248*2ab23a58SSong Liucontrol -- an ELF library's handling of empty sections, or a compiler version's 249*2ab23a58SSong Liunaming of anonymous data. A test which passes for you and skips for everyone 250*2ab23a58SSong Liuelse is worse than none. 251*2ab23a58SSong Liu 252*2ab23a58SSong Liu 253*2ab23a58SSong LiuChecklist 254*2ab23a58SSong Liu========= 255*2ab23a58SSong Liu 256*2ab23a58SSong LiuBefore committing: 257*2ab23a58SSong Liu 258*2ab23a58SSong Liu* the test fails with the code broken, and passes with it fixed 259*2ab23a58SSong Liu* the whole suite passes under **both** gcc and clang 260*2ab23a58SSong Liu* the premise is asserted, not assumed 261*2ab23a58SSong Liu* both directions are asserted where that applies 262*2ab23a58SSong Liu* no bare ``skip`` 263*2ab23a58SSong Liu* the fixture is in the same directory as the test 264*2ab23a58SSong Liu* the comment names the consequence, the fix commit, and anything load-bearing 265*2ab23a58SSong Liu* the commit contains one test and its fixtures, and nothing else 266*2ab23a58SSong Liu* the commit message says how you verified it 267