qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3] hw/i386: fix short-circuit logic with non-optimizing buil


From: Dan Hoffman
Subject: Re: [PATCH v3] hw/i386: fix short-circuit logic with non-optimizing builds
Date: Thu, 23 Nov 2023 12:03:16 -0600

I went ahead and wrote a clang-tidy pass that attempts to find other
cases of this behavior (i.e. compile-time short-circuit behavior that
could lead to undefined references). All of these cases should also be
caught by `-Wunreachable-code`, but that uncovers a lot and I'd like a
green light before I pursue that path (also some legitimate patterns
like file-local debug macro definitions will be flagged as warnings).
This check is intentionally broad (whether a symbol is defined is more
a property of the linker, so the compiler doesn't know).

I think this patch should be merged as-is (since it solves the
immediate functional issue I'm having) and I may submit a more
complete patch in the future that guards all references to
potentially-undefined KVM functions with `CONFIG_KVM_IS_POSSIBLE` or
something along those lines. Let me know what you think.

Here's some output from the clang-tidy pass against the current master branch.

```
../hw/i386/intel_iommu.c:4131:31: warning: function not declared in
this file is called from a short-circutable context
[misc-short-circuit-elision]
 4131 |         if (kvm_enabled() && !kvm_enable_x2apic()) {
      |                               ^
../hw/i386/x86.c:136:39: warning: function not declared in this file
is called from a short-circutable context [misc-short-circuit-elision]
  136 |         (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
      |                                       ^
../hw/i386/x86.c:422:27: warning: function not declared in this file
is called from a short-circutable context [misc-short-circuit-elision]
  422 |         kvm_enabled() && !kvm_hv_vpindex_settable()) {
      |                           ^
../hw/net/i82596.c:481:23: warning: function not declared in this file
is called from a short-circutable context [misc-short-circuit-elision]
  481 |     if (USE_TIMER && !timer_pending(s->flush_queue_timer)) {
      |                       ^
../hw/ppc/spapr.c:4543:27: warning: function not declared in this file
is called from a short-circutable context [misc-short-circuit-elision]
 4543 |     if (kvm_enabled() && !kvm_vcpu_id_is_valid(vcpu_id)) {
      |                           ^
../system/physmem.c:1913:27: warning: function not declared in this
file is called from a short-circutable context
[misc-short-circuit-elision]
 1913 |     if (kvm_enabled() && !kvm_has_sync_mmu()) {
      |                           ^
../target/i386/cpu.c:7125:27: warning: function not declared in this
file is called from a short-circutable context
[misc-short-circuit-elision]
 7125 |     if (kvm_enabled() && !kvm_hyperv_expand_features(cpu, errp)) {
      |                           ^
../target/s390x/diag.c:174:30: warning: function not declared in this
file is called from a short-circutable context
[misc-short-circuit-elision]
  174 |         if (kvm_enabled() && kvm_s390_get_hpage_1m()) {
      |                              ^
```

If any of you have any questions, feel free to let me know



reply via email to

[Prev in Thread] Current Thread [Next in Thread]