Project

General

Profile

Actions

Bug #10363

closed

Invalid implicit conversion from void to int

Added by Evgeny Novikov over 4 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Start date:
06/01/2020
Due date:
% Done:

0%

Estimated time:
Detected in build:
git
Platform:
Published in build:

Description

Although Frama-C fails for the attached source file, GCC can compile it without critical errors. There is such the error:

[kernel] /home/debian/klever-inst/klever/build bases/build-base-linux-4.18-x86_64-allmodconfig/Storage/var/lib/klever/workspace/Branches-and-Tags-Processing/linux-4.18/drivers/block/paride/frpw.c:55:8: Failure:
  invalid implicit conversion from void to int
[kernel] User Error: skipping file /home/debian/klever-inst/klever-work/native-scheduler/scheduler/jobs/7292d346-219c-4d78-b695-7712a530fff6/klever-core-work-dir/job/vtg/drivers/block/paride/frpw.ko/empty/weaver/frpw.c that has errors.
[kernel] Frama-C aborted: invalid user input.


Files

frpw.c (2.47 MB) frpw.c Evgeny Novikov, 06/01/2020 07:28 PM
i7300_idle.i.tar.gz (227 KB) i7300_idle.i.tar.gz Evgeny Novikov, 03/25/2022 06:59 PM
add_disk.i.tar.gz (297 KB) add_disk.i.tar.gz Evgeny Novikov, 03/26/2022 11:31 AM
epia.i.tar.gz (172 KB) epia.i.tar.gz Evgeny Novikov, 03/26/2022 11:50 AM
Actions #1

Updated by Evgeny Novikov over 2 years ago

I revealed this issue one more time, but now I built up a simple example:

int main(int argc, char *argv[])
{
    int res;

    res = (argc > 1 ? ({ (void)1; }) : (void)2, (int)3);

    return res;
}

You can compile this code and check that function main always returns 3, while CIL fails to parse it with the error as in the issue description. It looks like the internal compound statement "({ ... })" confuses CIL. If you will replace it with, say, "(void)1" it will work.

This is example is artificial. The original issue comes from substitution of macro r1(), say, in https://elixir.bootlin.com/linux/v5.5.19/source/drivers/block/paride/friq.c#L58. Hopefully there are just several drivers that do this.

Actions #2

Updated by Evgeny Novikov over 2 years ago

  • Assignee set to Mikhail Mandrykin
Actions #3

Updated by Mikhail Mandrykin over 2 years ago

  • Status changed from New to Open

Attempted a tentative fix in framac|34aba90f5efb. The reason for the failure is, in particular, the line TVoid _ when what' = ADrop -> (t', e') (* strange GNU thing *) in handling of casts that seems to be intentionally added to work-around some GNU incompatibility. In certain cases when the value of an expression is statically ignored (as in this case due to the , operator), the outer cast to void is dropped for some reason. Why this was at all necessary at some point is not clear to me, especially since the line was in the codebase starting right from the initial commit. So the fix just attempts to minimize the impact of this workaround by taking the constant/non-constant context into consideration: We disable the [strange GNU] compatibility in non-constant contexts. Additionally, it was necessary to allow conditional conversions between two void types

Actions #4

Updated by Evgeny Novikov over 2 years ago

  • Status changed from Open to Resolved

Thank you, I will test this and report results in a while.

Actions #5

Updated by Evgeny Novikov over 2 years ago

It seems that the fix is not valid in some cases. For instance, after running new Frama-C for the attached input file:

toplevel.opt "-no-autoload-plugins" "-no-findlib" "-kernel-log" "e:problem desc.txt" "-machdep" "gcc_x86_64" "-c11" "-kernel-warn-key" "CERT:MSC:38=active" "-remove-unused-inline-functions" "-remove-unused-static-functions" "-no-annot" "-no-single-return" "-fold-temp-vars" "-shrink-initializers" "-keep-logical-operators" "-aggressive-merging" "-print" "-print-lines" "-no-print-annot" "-ocode" "cil.i" "i7300_idle.i" 

I got the invalid CIL file containing several void temporary variables. GCC and CPAchecker fails to parse it:

$ gcc -fsyntax-only -x c cil.i 
.../i7300_idle.c: In function ‘i7300_idle_notifier’:
.../i7300_idle.c:463:12: error: variable or field ‘tmp’ declared void
...

Actions #6

Updated by Mikhail Mandrykin over 2 years ago

Ultimately reverted the framac|34aba90f and pushed an entirely different fix framac|8caa89de. Basically, instead of disabling the workaround in this case, now the solution is to actually reproduce the work-around even in case of a GNU-body ({ ...; (void) e; }), where the what = ADrop cannot be passed from the caller and so the special handling is added in yet another place (GNU-body handling). The possible side-effect is translating some extra code in the ignored GNU bodies that previously were entirely dropped, which potentially can expose some more bugs

Actions #7

Updated by Evgeny Novikov over 2 years ago

The quite large test suit detected only one issue. Frama-C fails at processing the attached source file with the following error:

[kernel] .../linux-stable/include/linux/rhashtable.h:616:4: Failure: 
  lvalue of type void: tmp_4

This source file could be handled by GCC without errors.

Actions #8

Updated by Evgeny Novikov over 2 years ago

Besides, I checked that even though the issue has gone for the simplified test case, it still exists for the original complicated source file. I attached the newer version. The corresponding error is as follows:

[kernel] .../linux-5.10.27/drivers/block/paride/epia.c:59:10: Failure: 
  invalid implicit conversion from void to int

Actions #9

Updated by Mikhail Mandrykin over 2 years ago

Pushed framac|5b8e240a. It just refines treatment of some void expressions within GNU_BODY and in the arguments of the ?: operator. Inside the body we seek to avoid creating temp vars if the result is dropped; inside the ?: operator we allow `void` expressions in either side if the result is dropped. The attached test cases now pass, however, in general the semantics of the fixes is quite lax. In particular, epia.c has a call to __udelay and (void) 0 as arguments to ?:, which makes calculating the resulting type of the operation dubious: it can be either naively treated as `void` (which is wrong if the expression is later used in some larger context, as a temporary of type void is prohibited) or (void) 0 can be heuristically treated as plain 0 of type int (as in the heuristic discussed earlier, which is wrong since an implicit conversion between `int` and `void` is also prohibited). Technically, it seems there should be no expressions of type void in C, but the GNU {...; statement } allows for such possibility, hence all the issues

Actions #10

Updated by Mikhail Mandrykin over 2 years ago

Partially reverted previous fixes in favor of a minimal change: framac|93c99dbf. Allowing an implicit cast between `void` and any type in case of an ignored result covers all the cases. I finally (after attempting many manual tests with GCC, LLVM and Frama-C) vaguely understand the logic behind all those cases: if the final result is ultimately dropped, a sub-expression of type void is treated as if it is of type int with all the corresponding implicit conversions or even as an expression of any type. However, the notion of "ultimately" is not very consistent: Frama-C accepted a computation (int)(x ? f() : (char *)0); (f is void) even before the patch, while GCC and LLVM failed on that, but a simpler x ? f() : (char *)0 is accepted by all of them. Also, x ? f() : (struct s) {.f = 0 } is accepted by LLVM and GCC, but not Frama-C (even after the minimal fix since the ignored call is treated as (int)0 elsewhere). The problem here is properly propagating the static approximation of the dropped flag, which is already different in Frama-C vs. GCC and LLVM. A really thorough fix would require consistently changing it everywhere, which is a lot of places in the entire expression translation function (about 2000 lines of code)

Actions #11

Updated by Mikhail Mandrykin over 2 years ago

After more tests: Actually, the behavior of GCC and LLVM can be explained simpler: the result of a conditional conversion between void and any other type is simply void. Then the value of type void must be ignored (other conversions and operations don't allow it). No dropped flags needed anywhere. However, in Frama-C there are many places implicitly treating ignored expressions as int (the dummy value 0): at least in function calls, in casts, in statement expressions. In general this still requires a thorough revamp of the overall approach in the entire Frama-C translation procedure (the ubiquitous what argument is probably just a generally wrong optimization)

Actions #12

Updated by Evgeny Novikov over 2 years ago

  • Status changed from Open to Closed

Despite Mikhail pointed out that there is still much work to do due to mismatches between compilers and Frama-C, his fixes helped to overcome the specified issue and there is no degradations. The considerable refactoring of Frama-C in this regard is a separate topic.

Actions

Also available in: Atom PDF