Skip to content

Commit bb25fd4

Browse files
torvaldsgregkh
authored andcommitted
drm/edid: fix objtool warning in drm_cvt_modes()
commit d652d5f upstream. Commit 991fcb7 ("drm/edid: Fix uninitialized variable in drm_cvt_modes()") just replaced one warning with another. The original warning about a possibly uninitialized variable was due to the compiler not being smart enough to see that the case statement actually enumerated all possible cases. And the initial fix was just to add a "default" case that had a single "unreachable()", just to tell the compiler that that situation cannot happen. However, that doesn't actually fix the fundamental reason for the problem: the compiler still doesn't see that the existing case statements enumerate all possibilities, so the compiler will still generate code to jump to that unreachable case statement. It just won't complain about an uninitialized variable any more. So now the compiler generates code to our inline asm marker that we told it would not fall through, and end end result is basically random. We have created a bridge to nowhere. And then, depending on the random details of just exactly what the compiler ends up doing, 'objtool' might end up complaining about the conditional branches (for conditions that cannot happen, and that thus will never be taken - but if the compiler was not smart enough to figure that out, we can't expect objtool to do so) going off in the weeds. So depending on how the compiler has laid out the result, you might see something like this: drivers/gpu/drm/drm_edid.o: warning: objtool: do_cvt_mode() falls through to next function drm_mode_detailed.isra.0() and now you have a truly inscrutable warning that makes no sense at all unless you start looking at whatever random code the compiler happened to generate for our bare "unreachable()" statement. IOW, don't use "unreachable()" unless you have an _active_ operation that generates code that actually makes it obvious that something is not reachable (ie an UD instruction or similar). Solve the "compiler isn't smart enough" problem by just marking one of the cases as "default", so that even when the compiler doesn't otherwise see that we've enumerated all cases, the compiler will feel happy and safe about there always being a valid case that initializes the 'width' variable. This also generates better code, since now the compiler doesn't generate comparisons for five different possibilities (the four real ones and the one that can't happen), but just for the three real ones and "the rest" (which is that last one). A smart enough compiler that sees that we cover all the cases won't care. Cc: Lyude Paul <[email protected]> Cc: Ilia Mirkin <[email protected]> Cc: Josh Poimboeuf <[email protected]> Cc: Peter Zijlstra <[email protected]> Signed-off-by: Linus Torvalds <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent c13edad commit bb25fd4

1 file changed

Lines changed: 2 additions & 2 deletions

File tree

drivers/gpu/drm/drm_edid.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3102,6 +3102,8 @@ static int drm_cvt_modes(struct drm_connector *connector,
31023102

31033103
height = (cvt->code[0] + ((cvt->code[1] & 0xf0) << 4) + 1) * 2;
31043104
switch (cvt->code[1] & 0x0c) {
3105+
/* default - because compiler doesn't see that we've enumerated all cases */
3106+
default:
31053107
case 0x00:
31063108
width = height * 4 / 3;
31073109
break;
@@ -3114,8 +3116,6 @@ static int drm_cvt_modes(struct drm_connector *connector,
31143116
case 0x0c:
31153117
width = height * 15 / 9;
31163118
break;
3117-
default:
3118-
unreachable();
31193119
}
31203120

31213121
for (j = 1; j < 5; j++) {

0 commit comments

Comments
 (0)