* [PATCH 01/10] tools/xl: Fix exit code for `xl vkbattach`
2021-02-12 15:39 [PATCH for-4.15 00/10] tools: Support to use abi-dumper on libraries Andrew Cooper
@ 2021-02-12 15:39 ` Andrew Cooper
2021-02-16 15:57 ` Ian Jackson
2021-02-16 16:25 ` Ian Jackson
2021-02-12 15:39 ` [PATCH 02/10] tools/libxg: Fix uninitialised variable in write_x86_cpu_policy_records() Andrew Cooper
` (8 subsequent siblings)
9 siblings, 2 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 15:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Anthony PERARD
Various version of gcc, when compiling with -Og, complain:
xl_vkb.c: In function 'main_vkbattach':
xl_vkb.c:79:12: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
79 | return rc;
| ^~
The dryrun_only path really does leave rc uninitalised. Introduce a done
label for success paths to use.
Fixes: a15166af7c3 ("xl: add vkb config parser and CLI")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
---
tools/xl/xl_vkb.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/xl/xl_vkb.c b/tools/xl/xl_vkb.c
index f6ed9e05ee..728ac9470b 100644
--- a/tools/xl/xl_vkb.c
+++ b/tools/xl/xl_vkb.c
@@ -64,7 +64,7 @@ int main_vkbattach(int argc, char **argv)
char *json = libxl_device_vkb_to_json(ctx, &vkb);
printf("vkb: %s\n", json);
free(json);
- goto out;
+ goto done;
}
if (libxl_device_vkb_add(ctx, domid, &vkb, 0)) {
@@ -72,6 +72,7 @@ int main_vkbattach(int argc, char **argv)
rc = ERROR_FAIL; goto out;
}
+done:
rc = 0;
out:
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 01/10] tools/xl: Fix exit code for `xl vkbattach`
2021-02-12 15:39 ` [PATCH 01/10] tools/xl: Fix exit code for `xl vkbattach` Andrew Cooper
@ 2021-02-16 15:57 ` Ian Jackson
2021-02-16 16:25 ` Ian Jackson
1 sibling, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 15:57 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Anthony PERARD
Andrew Cooper writes ("[PATCH 01/10] tools/xl: Fix exit code for `xl vkbattach`"):
> Various version of gcc, when compiling with -Og, complain:
>
> xl_vkb.c: In function 'main_vkbattach':
> xl_vkb.c:79:12: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 79 | return rc;
> | ^~
>
> The dryrun_only path really does leave rc uninitalised. Introduce a done
> label for success paths to use.
>
> Fixes: a15166af7c3 ("xl: add vkb config parser and CLI")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 01/10] tools/xl: Fix exit code for `xl vkbattach`
2021-02-12 15:39 ` [PATCH 01/10] tools/xl: Fix exit code for `xl vkbattach` Andrew Cooper
2021-02-16 15:57 ` Ian Jackson
@ 2021-02-16 16:25 ` Ian Jackson
1 sibling, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 16:25 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Anthony PERARD
Andrew Cooper writes ("[PATCH 01/10] tools/xl: Fix exit code for `xl vkbattach`"):
> Various version of gcc, when compiling with -Og, complain:
>
> xl_vkb.c: In function 'main_vkbattach':
> xl_vkb.c:79:12: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 79 | return rc;
> | ^~
>
> The dryrun_only path really does leave rc uninitalised. Introduce a done
> label for success paths to use.
>
> Fixes: a15166af7c3 ("xl: add vkb config parser and CLI")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Ian Jackson <iwj@xenproject.org>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 02/10] tools/libxg: Fix uninitialised variable in write_x86_cpu_policy_records()
2021-02-12 15:39 [PATCH for-4.15 00/10] tools: Support to use abi-dumper on libraries Andrew Cooper
2021-02-12 15:39 ` [PATCH 01/10] tools/xl: Fix exit code for `xl vkbattach` Andrew Cooper
@ 2021-02-12 15:39 ` Andrew Cooper
2021-02-12 15:54 ` Jan Beulich
2021-02-16 15:57 ` Ian Jackson
2021-02-12 15:39 ` [PATCH 03/10] tools/libxg: Fix uninitialised variable in meminit() Andrew Cooper
` (7 subsequent siblings)
9 siblings, 2 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 15:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu
Various version of gcc, when compiling with -Og, complain:
xg_sr_common_x86.c: In function 'write_x86_cpu_policy_records':
xg_sr_common_x86.c:92:12: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
92 | return rc;
| ^~
The complaint is legitimate, and can occur with unexpected behaviour of two
related hypercalls in combination with a libc which permits zero-length
malloc()s.
Have an explicit rc = 0 on the success path, and make the MSRs record error
handling consistent with the CPUID record before it.
Fixes: f6b2b8ec53d ("libxc/save: Write X86_{CPUID,MSR}_DATA records")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
---
tools/libs/guest/xg_sr_common_x86.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/libs/guest/xg_sr_common_x86.c b/tools/libs/guest/xg_sr_common_x86.c
index 6f12483907..3168c5485f 100644
--- a/tools/libs/guest/xg_sr_common_x86.c
+++ b/tools/libs/guest/xg_sr_common_x86.c
@@ -83,7 +83,13 @@ int write_x86_cpu_policy_records(struct xc_sr_context *ctx)
msrs.length = nr_msrs * sizeof(xen_msr_entry_t);
if ( msrs.length )
+ {
rc = write_record(ctx, &msrs);
+ if ( rc )
+ goto out;
+ }
+
+ rc = 0;
out:
free(cpuid.data);
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 02/10] tools/libxg: Fix uninitialised variable in write_x86_cpu_policy_records()
2021-02-12 15:39 ` [PATCH 02/10] tools/libxg: Fix uninitialised variable in write_x86_cpu_policy_records() Andrew Cooper
@ 2021-02-12 15:54 ` Jan Beulich
2021-02-16 15:57 ` Ian Jackson
1 sibling, 0 replies; 47+ messages in thread
From: Jan Beulich @ 2021-02-12 15:54 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Ian Jackson, Wei Liu, Xen-devel
On 12.02.2021 16:39, Andrew Cooper wrote:
> Various version of gcc, when compiling with -Og, complain:
>
> xg_sr_common_x86.c: In function 'write_x86_cpu_policy_records':
> xg_sr_common_x86.c:92:12: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 92 | return rc;
> | ^~
>
> The complaint is legitimate, and can occur with unexpected behaviour of two
> related hypercalls in combination with a libc which permits zero-length
> malloc()s.
>
> Have an explicit rc = 0 on the success path, and make the MSRs record error
> handling consistent with the CPUID record before it.
>
> Fixes: f6b2b8ec53d ("libxc/save: Write X86_{CPUID,MSR}_DATA records")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 02/10] tools/libxg: Fix uninitialised variable in write_x86_cpu_policy_records()
2021-02-12 15:39 ` [PATCH 02/10] tools/libxg: Fix uninitialised variable in write_x86_cpu_policy_records() Andrew Cooper
2021-02-12 15:54 ` Jan Beulich
@ 2021-02-16 15:57 ` Ian Jackson
1 sibling, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 15:57 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu
Andrew Cooper writes ("[PATCH 02/10] tools/libxg: Fix uninitialised variable in write_x86_cpu_policy_records()"):
> Fixes: f6b2b8ec53d ("libxc/save: Write X86_{CPUID,MSR}_DATA records")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 03/10] tools/libxg: Fix uninitialised variable in meminit()
2021-02-12 15:39 [PATCH for-4.15 00/10] tools: Support to use abi-dumper on libraries Andrew Cooper
2021-02-12 15:39 ` [PATCH 01/10] tools/xl: Fix exit code for `xl vkbattach` Andrew Cooper
2021-02-12 15:39 ` [PATCH 02/10] tools/libxg: Fix uninitialised variable in write_x86_cpu_policy_records() Andrew Cooper
@ 2021-02-12 15:39 ` Andrew Cooper
2021-02-12 15:55 ` Julien Grall
2021-02-12 20:01 ` [PATCH v1.1 03/10] tools/libxg: Drop stale p2m logic from ARM's meminit() Andrew Cooper
2021-02-12 15:39 ` [PATCH 04/10] tools/libxl: Fix uninitialised variable in libxl__domain_get_device_model_uid() Andrew Cooper
` (6 subsequent siblings)
9 siblings, 2 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 15:39 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Ian Jackson, Wei Liu, Stefano Stabellini, Julien Grall
Various version of gcc, when compiling with -Og, complain:
xg_dom_arm.c: In function 'meminit':
xg_dom_arm.c:420:19: error: 'p2m_size' may be used uninitialized in this function [-Werror=maybe-uninitialized]
420 | dom->p2m_size = p2m_size;
| ~~~~~~~~~~~~~~^~~~~~~~~~
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
Julien/Stefano: I can't work out how this variable is supposed to work, and
the fact that it isn't a straight accumulation across the RAM banks looks
suspect.
---
tools/libs/guest/xg_dom_arm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/libs/guest/xg_dom_arm.c b/tools/libs/guest/xg_dom_arm.c
index 94948d2b20..f1b8d06f75 100644
--- a/tools/libs/guest/xg_dom_arm.c
+++ b/tools/libs/guest/xg_dom_arm.c
@@ -373,7 +373,7 @@ static int meminit(struct xc_dom_image *dom)
const uint64_t modsize = dtb_size + ramdisk_size;
const uint64_t ram128mb = bankbase[0] + (128<<20);
- xen_pfn_t p2m_size;
+ xen_pfn_t p2m_size = 0;
uint64_t bank0end;
assert(dom->rambase_pfn << XC_PAGE_SHIFT == bankbase[0]);
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 03/10] tools/libxg: Fix uninitialised variable in meminit()
2021-02-12 15:39 ` [PATCH 03/10] tools/libxg: Fix uninitialised variable in meminit() Andrew Cooper
@ 2021-02-12 15:55 ` Julien Grall
2021-02-12 19:35 ` Andrew Cooper
2021-02-12 20:01 ` [PATCH v1.1 03/10] tools/libxg: Drop stale p2m logic from ARM's meminit() Andrew Cooper
1 sibling, 1 reply; 47+ messages in thread
From: Julien Grall @ 2021-02-12 15:55 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Ian Jackson, Wei Liu, Stefano Stabellini
Hi Andrew,
On 12/02/2021 15:39, Andrew Cooper wrote:
> Various version of gcc, when compiling with -Og, complain:
>
> xg_dom_arm.c: In function 'meminit':
> xg_dom_arm.c:420:19: error: 'p2m_size' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 420 | dom->p2m_size = p2m_size;
> | ~~~~~~~~~~~~~~^~~~~~~~~~
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
This was reported nearly 3 years ago (see [1]) and it is pretty sad this
was never merged :(.
> ---
> CC: Ian Jackson <iwj@xenproject.org>
> CC: Wei Liu <wl@xen.org>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
>
> Julien/Stefano: I can't work out how this variable is supposed to work, and
> the fact that it isn't a straight accumulation across the RAM banks looks
> suspect.
It looks buggy, but the P2M is never used on Arm. In fact, you sent a
patch a year ago to drop it (see [2]). It would be nice to revive it.
> ---
> tools/libs/guest/xg_dom_arm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/libs/guest/xg_dom_arm.c b/tools/libs/guest/xg_dom_arm.c
> index 94948d2b20..f1b8d06f75 100644
> --- a/tools/libs/guest/xg_dom_arm.c
> +++ b/tools/libs/guest/xg_dom_arm.c
> @@ -373,7 +373,7 @@ static int meminit(struct xc_dom_image *dom)
> const uint64_t modsize = dtb_size + ramdisk_size;
> const uint64_t ram128mb = bankbase[0] + (128<<20);
>
> - xen_pfn_t p2m_size;
> + xen_pfn_t p2m_size = 0;
> uint64_t bank0end;
>
> assert(dom->rambase_pfn << XC_PAGE_SHIFT == bankbase[0]);
>
If your original series is too risky for 4.15, I would consider to
remote p2m_size completely and always 0 dom->p2m_size.
Cheers,
[1]
https://lore.kernel.org/xen-devel/20180314123203.30646-1-wei.liu2@citrix.com/
[2]
https://patchwork.kernel.org/project/xen-devel/patch/20191217201550.15864-3-andrew.cooper3@citrix.com/
--
Julien Grall
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 03/10] tools/libxg: Fix uninitialised variable in meminit()
2021-02-12 15:55 ` Julien Grall
@ 2021-02-12 19:35 ` Andrew Cooper
0 siblings, 0 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 19:35 UTC (permalink / raw)
To: Julien Grall, Xen-devel; +Cc: Ian Jackson, Wei Liu, Stefano Stabellini
On 12/02/2021 15:55, Julien Grall wrote:
> Hi Andrew,
>
> On 12/02/2021 15:39, Andrew Cooper wrote:
>> Various version of gcc, when compiling with -Og, complain:
>>
>> xg_dom_arm.c: In function 'meminit':
>> xg_dom_arm.c:420:19: error: 'p2m_size' may be used uninitialized
>> in this function [-Werror=maybe-uninitialized]
>> 420 | dom->p2m_size = p2m_size;
>> | ~~~~~~~~~~~~~~^~~~~~~~~~
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> This was reported nearly 3 years ago (see [1]) and it is pretty sad
> this was never merged :(.
:( We've got far too many patches which fall through the cracks like this.
>
>> ---
>> CC: Ian Jackson <iwj@xenproject.org>
>> CC: Wei Liu <wl@xen.org>
>> CC: Stefano Stabellini <sstabellini@kernel.org>
>> CC: Julien Grall <julien@xen.org>
>>
>> Julien/Stefano: I can't work out how this variable is supposed to
>> work, and
>> the fact that it isn't a straight accumulation across the RAM banks
>> looks
>> suspect.
>
> It looks buggy, but the P2M is never used on Arm. In fact, you sent a
> patch a year ago to drop it (see [2]). It would be nice to revive it.
That series was committed more than a year ago - ee21f10d70^..97e34ad22d
- and tbh, I'd forgotten about it.
In light of that, I think I'll just delete the p2m_size references
here. It's easy to prove correctness via inspection, and removes a
dubious construct entirely.
~Andrew
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v1.1 03/10] tools/libxg: Drop stale p2m logic from ARM's meminit()
2021-02-12 15:39 ` [PATCH 03/10] tools/libxg: Fix uninitialised variable in meminit() Andrew Cooper
2021-02-12 15:55 ` Julien Grall
@ 2021-02-12 20:01 ` Andrew Cooper
2021-02-16 16:00 ` Ian Jackson
2021-02-16 16:27 ` Julien Grall
1 sibling, 2 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 20:01 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Ian Jackson, Wei Liu, Stefano Stabellini, Julien Grall
Various version of gcc, when compiling with -Og, complain:
xg_dom_arm.c: In function 'meminit':
xg_dom_arm.c:420:19: error: 'p2m_size' may be used uninitialized in this function [-Werror=maybe-uninitialized]
420 | dom->p2m_size = p2m_size;
| ~~~~~~~~~~~~~~^~~~~~~~~~
This is actually entirely stale code since ee21f10d70^..97e34ad22d which
removed the 1:1 identity p2m for translated domains.
Drop the write of d->p2m_size, and the p2m_size local variable. Reposition
the p2m_size field in struct xc_dom_image and correct some stale
documentation.
This change really ought to have been part of the original cleanup series.
No actual change to how ARM domains are constructed.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
v2:
* Delete stale p2m_size infrastructure.
---
tools/include/xenguest.h | 5 ++---
tools/libs/guest/xg_dom_arm.c | 5 -----
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/tools/include/xenguest.h b/tools/include/xenguest.h
index 775cf34c04..217022b6e7 100644
--- a/tools/include/xenguest.h
+++ b/tools/include/xenguest.h
@@ -145,6 +145,7 @@ struct xc_dom_image {
* eventually copied into guest context.
*/
xen_pfn_t *pv_p2m;
+ xen_pfn_t p2m_size; /* number of pfns covered by pv_p2m */
/* physical memory
*
@@ -154,12 +155,10 @@ struct xc_dom_image {
*
* An ARM guest has GUEST_RAM_BANKS regions of RAM, with
* rambank_size[i] pages in each. The lowest RAM address
- * (corresponding to the base of the p2m arrays above) is stored
- * in rambase_pfn.
+ * is stored in rambase_pfn.
*/
xen_pfn_t rambase_pfn;
xen_pfn_t total_pages;
- xen_pfn_t p2m_size; /* number of pfns covered by p2m */
struct xc_dom_phys *phys_pages;
#if defined (__arm__) || defined(__aarch64__)
xen_pfn_t rambank_size[GUEST_RAM_BANKS];
diff --git a/tools/libs/guest/xg_dom_arm.c b/tools/libs/guest/xg_dom_arm.c
index 94948d2b20..b4c24f15fb 100644
--- a/tools/libs/guest/xg_dom_arm.c
+++ b/tools/libs/guest/xg_dom_arm.c
@@ -373,7 +373,6 @@ static int meminit(struct xc_dom_image *dom)
const uint64_t modsize = dtb_size + ramdisk_size;
const uint64_t ram128mb = bankbase[0] + (128<<20);
- xen_pfn_t p2m_size;
uint64_t bank0end;
assert(dom->rambase_pfn << XC_PAGE_SHIFT == bankbase[0]);
@@ -409,16 +408,12 @@ static int meminit(struct xc_dom_image *dom)
ramsize -= banksize;
- p2m_size = ( bankbase[i] + banksize - bankbase[0] ) >> XC_PAGE_SHIFT;
-
dom->rambank_size[i] = banksize >> XC_PAGE_SHIFT;
}
assert(dom->rambank_size[0] != 0);
assert(ramsize == 0); /* Too much RAM is rejected above */
- dom->p2m_size = p2m_size;
-
/* setup initial p2m and allocate guest memory */
for ( i = 0; i < GUEST_RAM_BANKS && dom->rambank_size[i]; i++ )
{
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v1.1 03/10] tools/libxg: Drop stale p2m logic from ARM's meminit()
2021-02-12 20:01 ` [PATCH v1.1 03/10] tools/libxg: Drop stale p2m logic from ARM's meminit() Andrew Cooper
@ 2021-02-16 16:00 ` Ian Jackson
2021-02-16 16:27 ` Julien Grall
1 sibling, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 16:00 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Stefano Stabellini, Julien Grall
Andrew Cooper writes ("[PATCH v1.1 03/10] tools/libxg: Drop stale p2m logic from ARM's meminit()"):
> Various version of gcc, when compiling with -Og, complain:
>
> xg_dom_arm.c: In function 'meminit':
> xg_dom_arm.c:420:19: error: 'p2m_size' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 420 | dom->p2m_size = p2m_size;
> | ~~~~~~~~~~~~~~^~~~~~~~~~
>
> This is actually entirely stale code since ee21f10d70^..97e34ad22d which
> removed the 1:1 identity p2m for translated domains.
>
> Drop the write of d->p2m_size, and the p2m_size local variable. Reposition
> the p2m_size field in struct xc_dom_image and correct some stale
> documentation.
>
> This change really ought to have been part of the original cleanup series.
>
> No actual change to how ARM domains are constructed.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v1.1 03/10] tools/libxg: Drop stale p2m logic from ARM's meminit()
2021-02-12 20:01 ` [PATCH v1.1 03/10] tools/libxg: Drop stale p2m logic from ARM's meminit() Andrew Cooper
2021-02-16 16:00 ` Ian Jackson
@ 2021-02-16 16:27 ` Julien Grall
1 sibling, 0 replies; 47+ messages in thread
From: Julien Grall @ 2021-02-16 16:27 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Ian Jackson, Wei Liu, Stefano Stabellini
Hi Andrew,
On 12/02/2021 20:01, Andrew Cooper wrote:
> Various version of gcc, when compiling with -Og, complain:
>
> xg_dom_arm.c: In function 'meminit':
> xg_dom_arm.c:420:19: error: 'p2m_size' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 420 | dom->p2m_size = p2m_size;
> | ~~~~~~~~~~~~~~^~~~~~~~~~
>
> This is actually entirely stale code since ee21f10d70^..97e34ad22d which
> removed the 1:1 identity p2m for translated domains.
>
> Drop the write of d->p2m_size, and the p2m_size local variable. Reposition
> the p2m_size field in struct xc_dom_image and correct some stale
> documentation.
>
> This change really ought to have been part of the original cleanup series.
>
> No actual change to how ARM domains are constructed.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Cheers,
> ---
> CC: Ian Jackson <iwj@xenproject.org>
> CC: Wei Liu <wl@xen.org>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
>
> v2:
> * Delete stale p2m_size infrastructure.
> ---
> tools/include/xenguest.h | 5 ++---
> tools/libs/guest/xg_dom_arm.c | 5 -----
> 2 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/tools/include/xenguest.h b/tools/include/xenguest.h
> index 775cf34c04..217022b6e7 100644
> --- a/tools/include/xenguest.h
> +++ b/tools/include/xenguest.h
> @@ -145,6 +145,7 @@ struct xc_dom_image {
> * eventually copied into guest context.
> */
> xen_pfn_t *pv_p2m;
> + xen_pfn_t p2m_size; /* number of pfns covered by pv_p2m */
>
> /* physical memory
> *
> @@ -154,12 +155,10 @@ struct xc_dom_image {
> *
> * An ARM guest has GUEST_RAM_BANKS regions of RAM, with
> * rambank_size[i] pages in each. The lowest RAM address
> - * (corresponding to the base of the p2m arrays above) is stored
> - * in rambase_pfn.
> + * is stored in rambase_pfn.
> */
> xen_pfn_t rambase_pfn;
> xen_pfn_t total_pages;
> - xen_pfn_t p2m_size; /* number of pfns covered by p2m */
> struct xc_dom_phys *phys_pages;
> #if defined (__arm__) || defined(__aarch64__)
> xen_pfn_t rambank_size[GUEST_RAM_BANKS];
> diff --git a/tools/libs/guest/xg_dom_arm.c b/tools/libs/guest/xg_dom_arm.c
> index 94948d2b20..b4c24f15fb 100644
> --- a/tools/libs/guest/xg_dom_arm.c
> +++ b/tools/libs/guest/xg_dom_arm.c
> @@ -373,7 +373,6 @@ static int meminit(struct xc_dom_image *dom)
> const uint64_t modsize = dtb_size + ramdisk_size;
> const uint64_t ram128mb = bankbase[0] + (128<<20);
>
> - xen_pfn_t p2m_size;
> uint64_t bank0end;
>
> assert(dom->rambase_pfn << XC_PAGE_SHIFT == bankbase[0]);
> @@ -409,16 +408,12 @@ static int meminit(struct xc_dom_image *dom)
>
> ramsize -= banksize;
>
> - p2m_size = ( bankbase[i] + banksize - bankbase[0] ) >> XC_PAGE_SHIFT;
> -
> dom->rambank_size[i] = banksize >> XC_PAGE_SHIFT;
> }
>
> assert(dom->rambank_size[0] != 0);
> assert(ramsize == 0); /* Too much RAM is rejected above */
>
> - dom->p2m_size = p2m_size;
> -
> /* setup initial p2m and allocate guest memory */
> for ( i = 0; i < GUEST_RAM_BANKS && dom->rambank_size[i]; i++ )
> {
>
--
Julien Grall
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 04/10] tools/libxl: Fix uninitialised variable in libxl__domain_get_device_model_uid()
2021-02-12 15:39 [PATCH for-4.15 00/10] tools: Support to use abi-dumper on libraries Andrew Cooper
` (2 preceding siblings ...)
2021-02-12 15:39 ` [PATCH 03/10] tools/libxg: Fix uninitialised variable in meminit() Andrew Cooper
@ 2021-02-12 15:39 ` Andrew Cooper
2021-02-16 16:07 ` Ian Jackson
2021-02-16 17:43 ` [PATCH v2 " Andrew Cooper
2021-02-12 15:39 ` [PATCH 05/10] tools/libxl: Fix uninitialised variable in libxl__write_stub_dmargs() Andrew Cooper
` (5 subsequent siblings)
9 siblings, 2 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 15:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Anthony PERARD
Various version of gcc, when compiling with -Og, complain:
libxl_dm.c: In function 'libxl__domain_get_device_model_uid':
libxl_dm.c:256:12: error: 'kill_by_uid' may be used uninitialized in this function [-Werror=maybe-uninitialized]
256 | if (kill_by_uid)
| ^
The logic is sufficiently complicated I can't figure out if the complain is
legitimate or not. There is exactly one path wanting kill_by_uid set to true,
so default it to false and drop the existing workaround for this problem at
other optimisation levels.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libs/light/libxl_dm.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 291dee9b3f..1ca21e4b81 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -128,7 +128,7 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc,
int rc;
char *user;
uid_t intended_uid = -1;
- bool kill_by_uid;
+ bool kill_by_uid = false;
/* Only qemu-upstream can run as a different uid */
if (b_info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN)
@@ -176,7 +176,6 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc,
LOGD(DEBUG, guest_domid,
"dm_restrict disabled, starting QEMU as root");
user = NULL; /* Should already be null, but just in case */
- kill_by_uid = false; /* Keep older versions of gcc happy */
rc = 0;
goto out;
}
@@ -227,7 +226,6 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc,
LOGD(WARN, guest_domid, "Could not find user %s, falling back to %s",
LIBXL_QEMU_USER_RANGE_BASE, LIBXL_QEMU_USER_SHARED);
intended_uid = user_base->pw_uid;
- kill_by_uid = false;
rc = 0;
goto out;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 04/10] tools/libxl: Fix uninitialised variable in libxl__domain_get_device_model_uid()
2021-02-12 15:39 ` [PATCH 04/10] tools/libxl: Fix uninitialised variable in libxl__domain_get_device_model_uid() Andrew Cooper
@ 2021-02-16 16:07 ` Ian Jackson
2021-02-16 17:43 ` [PATCH v2 " Andrew Cooper
1 sibling, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 16:07 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Ian Jackson, Wei Liu, Anthony PERARD
Andrew Cooper writes ("[PATCH 04/10] tools/libxl: Fix uninitialised variable in libxl__domain_get_device_model_uid()"):
> The logic is sufficiently complicated I can't figure out if the complain is
> legitimate or not. There is exactly one path wanting kill_by_uid set to true,
> so default it to false and drop the existing workaround for this problem at
> other optimisation levels.
The place where it's used is here:
if (!rc && user) {
state->dm_runas = user;
if (kill_by_uid)
state->dm_kill_uid = GCSPRINTF("%ld",...
This is gated by !rc. So for this to be used uninitialised, we'd have
to get here with rc==0 but uninitialised kill_by_uid.
The label `out` is preceded by a nonzero assignment to rc.
All the `goto out` are preceded by either (i) nonzero assignment to
rc, or (ii) assignment to kill_by_uid and setting rc=0.
So the compiler is wrong.
If only we had sum types.
In the absence of sum types I suggest the following restructuring:
Change all the `rc = ERROR...; goto out;` to `goto err` and make `goto
out` be the success path only.
Ian.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v2 04/10] tools/libxl: Fix uninitialised variable in libxl__domain_get_device_model_uid()
2021-02-12 15:39 ` [PATCH 04/10] tools/libxl: Fix uninitialised variable in libxl__domain_get_device_model_uid() Andrew Cooper
2021-02-16 16:07 ` Ian Jackson
@ 2021-02-16 17:43 ` Andrew Cooper
2021-02-16 17:57 ` Ian Jackson
1 sibling, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2021-02-16 17:43 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Anthony PERARD
Various version of gcc, when compiling with -Og, complain:
libxl_dm.c: In function 'libxl__domain_get_device_model_uid':
libxl_dm.c:256:12: error: 'kill_by_uid' may be used uninitialized in this function [-Werror=maybe-uninitialized]
256 | if (kill_by_uid)
| ^
The logic is very tangled. Split the out and err labels apart, using out for
success cases only.
assert() that rc is 0 in the success case. This allows for the removal of the
`if (!rc)` nesting in the out path, which reduces the cyclomatic complexity,
which is the root cause of false positive maybe-uninitialised warnings.
This also allows for dropping of the two paths setting kill_by_uid to false to
work around this warning at other optimisation levels.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
v2:
* Split the out and err paths.
---
tools/libs/light/libxl_dm.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 291dee9b3f..dbd2ab607d 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -135,8 +135,8 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc,
return 0;
/*
- * From this point onward, all paths should go through the `out`
- * label. The invariants should be:
+ * From this point onward, all paths should go through the `out` (success)
+ * or `err` (failure) labels. The invariants should be:
* - rc may be 0, or an error code.
* - if rc is an error code, user and intended_uid are ignored.
* - if rc is 0, user may be set or not set.
@@ -153,13 +153,13 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc,
if (user) {
rc = userlookup_helper_getpwnam(gc, user, &user_pwbuf, &user_base);
if (rc)
- goto out;
+ goto err;
if (!user_base) {
LOGD(ERROR, guest_domid, "Couldn't find device_model_user %s",
user);
rc = ERROR_INVAL;
- goto out;
+ goto err;
}
intended_uid = user_base->pw_uid;
@@ -176,7 +176,6 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc,
LOGD(DEBUG, guest_domid,
"dm_restrict disabled, starting QEMU as root");
user = NULL; /* Should already be null, but just in case */
- kill_by_uid = false; /* Keep older versions of gcc happy */
rc = 0;
goto out;
}
@@ -188,7 +187,7 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc,
rc = userlookup_helper_getpwnam(gc, LIBXL_QEMU_USER_RANGE_BASE,
&user_pwbuf, &user_base);
if (rc)
- goto out;
+ goto err;
if (user_base) {
struct passwd *user_clash, user_clash_pwbuf;
@@ -196,14 +195,14 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc,
rc = userlookup_helper_getpwuid(gc, intended_uid,
&user_clash_pwbuf, &user_clash);
if (rc)
- goto out;
+ goto err;
if (user_clash) {
LOGD(ERROR, guest_domid,
"wanted to use uid %ld (%s + %d) but that is user %s !",
(long)intended_uid, LIBXL_QEMU_USER_RANGE_BASE,
guest_domid, user_clash->pw_name);
rc = ERROR_INVAL;
- goto out;
+ goto err;
}
LOGD(DEBUG, guest_domid, "using uid %ld", (long)intended_uid);
@@ -222,12 +221,11 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc,
user = LIBXL_QEMU_USER_SHARED;
rc = userlookup_helper_getpwnam(gc, user, &user_pwbuf, &user_base);
if (rc)
- goto out;
+ goto err;
if (user_base) {
LOGD(WARN, guest_domid, "Could not find user %s, falling back to %s",
LIBXL_QEMU_USER_RANGE_BASE, LIBXL_QEMU_USER_SHARED);
intended_uid = user_base->pw_uid;
- kill_by_uid = false;
rc = 0;
goto out;
}
@@ -240,23 +238,26 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc,
"Could not find user %s or range base pseudo-user %s, cannot restrict",
LIBXL_QEMU_USER_SHARED, LIBXL_QEMU_USER_RANGE_BASE);
rc = ERROR_INVAL;
+ goto err;
out:
+ assert(rc == 0);
+
/* First, do a root check if appropriate */
- if (!rc) {
- if (user && intended_uid == 0) {
- LOGD(ERROR, guest_domid, "intended_uid is 0 (root)!");
- rc = ERROR_INVAL;
- }
+ if (user && intended_uid == 0) {
+ LOGD(ERROR, guest_domid, "intended_uid is 0 (root)!");
+ rc = ERROR_INVAL;
+ goto err;
}
/* Then do the final set, if still appropriate */
- if (!rc && user) {
+ if (user) {
state->dm_runas = user;
if (kill_by_uid)
state->dm_kill_uid = GCSPRINTF("%ld", (long)intended_uid);
}
+ err:
return rc;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v2 04/10] tools/libxl: Fix uninitialised variable in libxl__domain_get_device_model_uid()
2021-02-16 17:43 ` [PATCH v2 " Andrew Cooper
@ 2021-02-16 17:57 ` Ian Jackson
2021-02-16 18:05 ` Ian Jackson
0 siblings, 1 reply; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 17:57 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Anthony PERARD
Andrew Cooper writes ("[PATCH v2 04/10] tools/libxl: Fix uninitialised variable in libxl__domain_get_device_model_uid()"):
> Various version of gcc, when compiling with -Og, complain:
>
> libxl_dm.c: In function 'libxl__domain_get_device_model_uid':
> libxl_dm.c:256:12: error: 'kill_by_uid' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 256 | if (kill_by_uid)
...
> @@ -176,7 +176,6 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc,
> LOGD(DEBUG, guest_domid,
> "dm_restrict disabled, starting QEMU as root");
> user = NULL; /* Should already be null, but just in case */
> - kill_by_uid = false; /* Keep older versions of gcc happy */
> rc = 0;
> goto out;
Uhhhhh. I think this and the other one seem to be stray hunks which
each introduce a new uninitialised variable bug ?
Isn.
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2 04/10] tools/libxl: Fix uninitialised variable in libxl__domain_get_device_model_uid()
2021-02-16 17:57 ` Ian Jackson
@ 2021-02-16 18:05 ` Ian Jackson
0 siblings, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 18:05 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel, Wei Liu, Anthony PERARD
Ian Jackson writes ("Re: [PATCH v2 04/10] tools/libxl: Fix uninitialised variable in libxl__domain_get_device_model_uid()"):
> Uhhhhh. I think this and the other one seem to be stray hunks which
> each introduce a new uninitialised variable bug ?
I now think (following irc discussion) that I was wrong about this.
Use of intended_uid is conditional on user being set. This is very
confusing. This code is simulating a sum type. If only we had sum
types.
Reviewed-by: Ian Jackson <iwj@xenproject.org>
I think, given how confusing this is, that I would like another
careful review before I give my release-ack.
Ian.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 05/10] tools/libxl: Fix uninitialised variable in libxl__write_stub_dmargs()
2021-02-12 15:39 [PATCH for-4.15 00/10] tools: Support to use abi-dumper on libraries Andrew Cooper
` (3 preceding siblings ...)
2021-02-12 15:39 ` [PATCH 04/10] tools/libxl: Fix uninitialised variable in libxl__domain_get_device_model_uid() Andrew Cooper
@ 2021-02-12 15:39 ` Andrew Cooper
2021-02-16 16:07 ` Ian Jackson
2021-02-16 16:26 ` Ian Jackson
2021-02-12 15:39 ` [PATCH 06/10] stubdom/xenstored: Fix uninitialised variables in lu_read_state() Andrew Cooper
` (4 subsequent siblings)
9 siblings, 2 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 15:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Anthony PERARD
Various version of gcc, when compiling with -Og, complain:
libxl_dm.c: In function ‘libxl__write_stub_dmargs’:
libxl_dm.c:2166:16: error: ‘dmargs’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
rc = libxl__xs_write_checked(gc, t, path, dmargs);
~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
It can't, but only because of how the is_linux_stubdom checks line up.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libs/light/libxl_dm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 1ca21e4b81..7bbb8792ea 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -2099,7 +2099,7 @@ static int libxl__write_stub_dmargs(libxl__gc *gc,
{
struct xs_permissions roperm[2];
xs_transaction_t t = XBT_NULL;
- char *dmargs;
+ char *dmargs = NULL;
int rc;
roperm[0].id = 0;
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 05/10] tools/libxl: Fix uninitialised variable in libxl__write_stub_dmargs()
2021-02-12 15:39 ` [PATCH 05/10] tools/libxl: Fix uninitialised variable in libxl__write_stub_dmargs() Andrew Cooper
@ 2021-02-16 16:07 ` Ian Jackson
2021-02-16 16:26 ` Ian Jackson
1 sibling, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 16:07 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Anthony PERARD
Andrew Cooper writes ("[PATCH 05/10] tools/libxl: Fix uninitialised variable in libxl__write_stub_dmargs()"):
> Various version of gcc, when compiling with -Og, complain:
>
> libxl_dm.c: In function ‘libxl__write_stub_dmargs’:
> libxl_dm.c:2166:16: error: ‘dmargs’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> rc = libxl__xs_write_checked(gc, t, path, dmargs);
> ~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> It can't, but only because of how the is_linux_stubdom checks line up.
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 05/10] tools/libxl: Fix uninitialised variable in libxl__write_stub_dmargs()
2021-02-12 15:39 ` [PATCH 05/10] tools/libxl: Fix uninitialised variable in libxl__write_stub_dmargs() Andrew Cooper
2021-02-16 16:07 ` Ian Jackson
@ 2021-02-16 16:26 ` Ian Jackson
1 sibling, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 16:26 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Anthony PERARD
Andrew Cooper writes ("[PATCH 05/10] tools/libxl: Fix uninitialised variable in libxl__write_stub_dmargs()"):
> Various version of gcc, when compiling with -Og, complain:
>
> libxl_dm.c: In function ‘libxl__write_stub_dmargs’:
> libxl_dm.c:2166:16: error: ‘dmargs’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> rc = libxl__xs_write_checked(gc, t, path, dmargs);
> ~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Reviewed-by: Ian Jackson <iwj@xenproject.org>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 06/10] stubdom/xenstored: Fix uninitialised variables in lu_read_state()
2021-02-12 15:39 [PATCH for-4.15 00/10] tools: Support to use abi-dumper on libraries Andrew Cooper
` (4 preceding siblings ...)
2021-02-12 15:39 ` [PATCH 05/10] tools/libxl: Fix uninitialised variable in libxl__write_stub_dmargs() Andrew Cooper
@ 2021-02-12 15:39 ` Andrew Cooper
2021-02-12 16:08 ` Jürgen Groß
` (2 more replies)
2021-02-12 15:39 ` [PATCH 07/10] tools: Use -Og for debug builds when available Andrew Cooper
` (3 subsequent siblings)
9 siblings, 3 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 15:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Juergen Gross
Various version of gcc, when compiling with -Og, complain:
xenstored_control.c: In function ‘lu_read_state’:
xenstored_control.c:540:11: error: ‘state.size’ is used uninitialized in this
function [-Werror=uninitialized]
if (state.size == 0)
~~~~~^~~~~
xenstored_control.c:543:6: error: ‘state.buf’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
pre = state.buf;
~~~~^~~~~~~~~~~
xenstored_control.c:550:23: error: ‘state.buf’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
(void *)head - state.buf < state.size;
~~~~~^~~~
xenstored_control.c:550:35: error: ‘state.size’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
(void *)head - state.buf < state.size;
~~~~~^~~~~
Interestingly, this is only in the stubdom build. I can't identify any
relevant differences vs the regular tools build.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Juergen Gross <jgross@suse.com>
---
tools/xenstore/xenstored_control.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index 1f733e0a04..f10beaf85e 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -530,7 +530,7 @@ static const char *lu_dump_state(const void *ctx, struct connection *conn)
void lu_read_state(void)
{
- struct lu_dump_state state;
+ struct lu_dump_state state = {};
struct xs_state_record_header *head;
void *ctx = talloc_new(NULL); /* Work context for subfunctions. */
struct xs_state_preamble *pre;
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 06/10] stubdom/xenstored: Fix uninitialised variables in lu_read_state()
2021-02-12 15:39 ` [PATCH 06/10] stubdom/xenstored: Fix uninitialised variables in lu_read_state() Andrew Cooper
@ 2021-02-12 16:08 ` Jürgen Groß
2021-02-12 17:01 ` Andrew Cooper
2021-02-15 15:36 ` [PATCH v1.1 " Andrew Cooper
2021-02-16 16:10 ` [PATCH " Ian Jackson
2 siblings, 1 reply; 47+ messages in thread
From: Jürgen Groß @ 2021-02-12 16:08 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Ian Jackson, Wei Liu
[-- Attachment #1.1.1: Type: text/plain, Size: 1447 bytes --]
On 12.02.21 16:39, Andrew Cooper wrote:
> Various version of gcc, when compiling with -Og, complain:
>
> xenstored_control.c: In function ‘lu_read_state’:
> xenstored_control.c:540:11: error: ‘state.size’ is used uninitialized in this
> function [-Werror=uninitialized]
> if (state.size == 0)
> ~~~~~^~~~~
> xenstored_control.c:543:6: error: ‘state.buf’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
> pre = state.buf;
> ~~~~^~~~~~~~~~~
> xenstored_control.c:550:23: error: ‘state.buf’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
> (void *)head - state.buf < state.size;
> ~~~~~^~~~
> xenstored_control.c:550:35: error: ‘state.size’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
> (void *)head - state.buf < state.size;
> ~~~~~^~~~~
>
> Interestingly, this is only in the stubdom build. I can't identify any
> relevant differences vs the regular tools build.
But me. :-)
lu_get_dump_state() is empty for the stubdom case (this will change when
LU is implemented for stubdom, too). In the daemon case this function is
setting all the fields which are relevant.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Juergen
[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 06/10] stubdom/xenstored: Fix uninitialised variables in lu_read_state()
2021-02-12 16:08 ` Jürgen Groß
@ 2021-02-12 17:01 ` Andrew Cooper
2021-02-13 9:36 ` Jürgen Groß
0 siblings, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 17:01 UTC (permalink / raw)
To: Jürgen Groß, Xen-devel; +Cc: Ian Jackson, Wei Liu
On 12/02/2021 16:08, Jürgen Groß wrote:
> On 12.02.21 16:39, Andrew Cooper wrote:
>> Various version of gcc, when compiling with -Og, complain:
>>
>> xenstored_control.c: In function ‘lu_read_state’:
>> xenstored_control.c:540:11: error: ‘state.size’ is used
>> uninitialized in this
>> function [-Werror=uninitialized]
>> if (state.size == 0)
>> ~~~~~^~~~~
>> xenstored_control.c:543:6: error: ‘state.buf’ may be used
>> uninitialized in
>> this function [-Werror=maybe-uninitialized]
>> pre = state.buf;
>> ~~~~^~~~~~~~~~~
>> xenstored_control.c:550:23: error: ‘state.buf’ may be used
>> uninitialized in
>> this function [-Werror=maybe-uninitialized]
>> (void *)head - state.buf < state.size;
>> ~~~~~^~~~
>> xenstored_control.c:550:35: error: ‘state.size’ may be used
>> uninitialized in
>> this function [-Werror=maybe-uninitialized]
>> (void *)head - state.buf < state.size;
>> ~~~~~^~~~~
>>
>> Interestingly, this is only in the stubdom build. I can't identify any
>> relevant differences vs the regular tools build.
>
> But me. :-)
>
> lu_get_dump_state() is empty for the stubdom case (this will change when
> LU is implemented for stubdom, too). In the daemon case this function is
> setting all the fields which are relevant.
So I spotted that. This instance of lu_read_state() is already within
the ifdefary, so doesn't get to see the empty stub (I think).
Also, I'd expect the compiler to complain at -O2 if it spotted that code.
>
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Reviewed-by: Juergen Gross <jgross@suse.com>
Thanks,
~Andrew
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 06/10] stubdom/xenstored: Fix uninitialised variables in lu_read_state()
2021-02-12 17:01 ` Andrew Cooper
@ 2021-02-13 9:36 ` Jürgen Groß
2021-02-15 14:12 ` Andrew Cooper
0 siblings, 1 reply; 47+ messages in thread
From: Jürgen Groß @ 2021-02-13 9:36 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Ian Jackson, Wei Liu
[-- Attachment #1.1.1: Type: text/plain, Size: 1812 bytes --]
On 12.02.21 18:01, Andrew Cooper wrote:
> On 12/02/2021 16:08, Jürgen Groß wrote:
>> On 12.02.21 16:39, Andrew Cooper wrote:
>>> Various version of gcc, when compiling with -Og, complain:
>>>
>>> xenstored_control.c: In function ‘lu_read_state’:
>>> xenstored_control.c:540:11: error: ‘state.size’ is used
>>> uninitialized in this
>>> function [-Werror=uninitialized]
>>> if (state.size == 0)
>>> ~~~~~^~~~~
>>> xenstored_control.c:543:6: error: ‘state.buf’ may be used
>>> uninitialized in
>>> this function [-Werror=maybe-uninitialized]
>>> pre = state.buf;
>>> ~~~~^~~~~~~~~~~
>>> xenstored_control.c:550:23: error: ‘state.buf’ may be used
>>> uninitialized in
>>> this function [-Werror=maybe-uninitialized]
>>> (void *)head - state.buf < state.size;
>>> ~~~~~^~~~
>>> xenstored_control.c:550:35: error: ‘state.size’ may be used
>>> uninitialized in
>>> this function [-Werror=maybe-uninitialized]
>>> (void *)head - state.buf < state.size;
>>> ~~~~~^~~~~
>>>
>>> Interestingly, this is only in the stubdom build. I can't identify any
>>> relevant differences vs the regular tools build.
>>
>> But me. :-)
>>
>> lu_get_dump_state() is empty for the stubdom case (this will change when
>> LU is implemented for stubdom, too). In the daemon case this function is
>> setting all the fields which are relevant.
>
> So I spotted that. This instance of lu_read_state() is already within
> the ifdefary, so doesn't get to see the empty stub (I think).
There is only one instance of lu_read_state().
Juergen
[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 06/10] stubdom/xenstored: Fix uninitialised variables in lu_read_state()
2021-02-13 9:36 ` Jürgen Groß
@ 2021-02-15 14:12 ` Andrew Cooper
0 siblings, 0 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-15 14:12 UTC (permalink / raw)
To: Jürgen Groß, Xen-devel; +Cc: Ian Jackson, Wei Liu
On 13/02/2021 09:36, Jürgen Groß wrote:
> On 12.02.21 18:01, Andrew Cooper wrote:
>> On 12/02/2021 16:08, Jürgen Groß wrote:
>>> On 12.02.21 16:39, Andrew Cooper wrote:
>>>> Various version of gcc, when compiling with -Og, complain:
>>>>
>>>> xenstored_control.c: In function ‘lu_read_state’:
>>>> xenstored_control.c:540:11: error: ‘state.size’ is used
>>>> uninitialized in this
>>>> function [-Werror=uninitialized]
>>>> if (state.size == 0)
>>>> ~~~~~^~~~~
>>>> xenstored_control.c:543:6: error: ‘state.buf’ may be used
>>>> uninitialized in
>>>> this function [-Werror=maybe-uninitialized]
>>>> pre = state.buf;
>>>> ~~~~^~~~~~~~~~~
>>>> xenstored_control.c:550:23: error: ‘state.buf’ may be used
>>>> uninitialized in
>>>> this function [-Werror=maybe-uninitialized]
>>>> (void *)head - state.buf < state.size;
>>>> ~~~~~^~~~
>>>> xenstored_control.c:550:35: error: ‘state.size’ may be used
>>>> uninitialized in
>>>> this function [-Werror=maybe-uninitialized]
>>>> (void *)head - state.buf < state.size;
>>>> ~~~~~^~~~~
>>>>
>>>> Interestingly, this is only in the stubdom build. I can't identify
>>>> any
>>>> relevant differences vs the regular tools build.
>>>
>>> But me. :-)
>>>
>>> lu_get_dump_state() is empty for the stubdom case (this will change
>>> when
>>> LU is implemented for stubdom, too). In the daemon case this
>>> function is
>>> setting all the fields which are relevant.
>>
>> So I spotted that. This instance of lu_read_state() is already within
>> the ifdefary, so doesn't get to see the empty stub (I think).
>
> There is only one instance of lu_read_state().
Ahh - I got the NO_LIVE_UPDATE and __MINIOS__ ifdefary inverted. (It is
very confusing to follow).
I'll reword the commit message, but leave the fix the same.
~Andrew
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v1.1 06/10] stubdom/xenstored: Fix uninitialised variables in lu_read_state()
2021-02-12 15:39 ` [PATCH 06/10] stubdom/xenstored: Fix uninitialised variables in lu_read_state() Andrew Cooper
2021-02-12 16:08 ` Jürgen Groß
@ 2021-02-15 15:36 ` Andrew Cooper
2021-02-16 16:10 ` [PATCH " Ian Jackson
2 siblings, 0 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-15 15:36 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Juergen Gross
Various version of gcc, when compiling with -Og, complain:
xenstored_control.c: In function ‘lu_read_state’:
xenstored_control.c:540:11: error: ‘state.size’ is used uninitialized in this
function [-Werror=uninitialized]
if (state.size == 0)
~~~~~^~~~~
xenstored_control.c:543:6: error: ‘state.buf’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
pre = state.buf;
~~~~^~~~~~~~~~~
xenstored_control.c:550:23: error: ‘state.buf’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
(void *)head - state.buf < state.size;
~~~~~^~~~
xenstored_control.c:550:35: error: ‘state.size’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
(void *)head - state.buf < state.size;
~~~~~^~~~~
for the stubdom build. This is because lu_get_dump_state() is a no-op stub in
MiniOS, and state really is operated on uninitialised.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Juergen Gross <jgross@suse.com>
v2:
* Reword commit message.
---
tools/xenstore/xenstored_control.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index 1f733e0a04..f10beaf85e 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -530,7 +530,7 @@ static const char *lu_dump_state(const void *ctx, struct connection *conn)
void lu_read_state(void)
{
- struct lu_dump_state state;
+ struct lu_dump_state state = {};
struct xs_state_record_header *head;
void *ctx = talloc_new(NULL); /* Work context for subfunctions. */
struct xs_state_preamble *pre;
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 06/10] stubdom/xenstored: Fix uninitialised variables in lu_read_state()
2021-02-12 15:39 ` [PATCH 06/10] stubdom/xenstored: Fix uninitialised variables in lu_read_state() Andrew Cooper
2021-02-12 16:08 ` Jürgen Groß
2021-02-15 15:36 ` [PATCH v1.1 " Andrew Cooper
@ 2021-02-16 16:10 ` Ian Jackson
2 siblings, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 16:10 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Juergen Gross
Andrew Cooper writes ("[PATCH 06/10] stubdom/xenstored: Fix uninitialised variables in lu_read_state()"):
> Various version of gcc, when compiling with -Og, complain:
>
> xenstored_control.c: In function ‘lu_read_state’:
> xenstored_control.c:540:11: error: ‘state.size’ is used uninitialized in this
> function [-Werror=uninitialized]
> if (state.size == 0)
> ~~~~~^~~~~
> xenstored_control.c:543:6: error: ‘state.buf’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
> pre = state.buf;
> ~~~~^~~~~~~~~~~
> xenstored_control.c:550:23: error: ‘state.buf’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
> (void *)head - state.buf < state.size;
> ~~~~~^~~~
> xenstored_control.c:550:35: error: ‘state.size’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
> (void *)head - state.buf < state.size;
> ~~~~~^~~~~
>
> Interestingly, this is only in the stubdom build. I can't identify any
> relevant differences vs the regular tools build.
#ifdef __MINIOS__
static void lu_get_dump_state(struct lu_dump_state *state)
{
}
So the compiler is right to complain that
lu_get_dump_state(&state);
if (state.size == 0)
barf_perror("No state found after live-update");
this will use state.size uninitialised.
It's probably just luck that this works at all, if it does,
anywhere...
Reviewed-by: Ian Jackson <iwj@xenproject.org>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
Ian.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 07/10] tools: Use -Og for debug builds when available
2021-02-12 15:39 [PATCH for-4.15 00/10] tools: Support to use abi-dumper on libraries Andrew Cooper
` (5 preceding siblings ...)
2021-02-12 15:39 ` [PATCH 06/10] stubdom/xenstored: Fix uninitialised variables in lu_read_state() Andrew Cooper
@ 2021-02-12 15:39 ` Andrew Cooper
2021-02-12 16:04 ` Jan Beulich
2021-02-16 16:11 ` Ian Jackson
2021-02-12 15:39 ` [PATCH 08/10] tools: Check for abi-dumper in ./configure Andrew Cooper
` (2 subsequent siblings)
9 siblings, 2 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 15:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Juergen Gross
The recommended optimisation level for debugging is -Og, and is what tools
such as gdb prefer. In practice, it equates to -01 with a few specific
optimisations turned off.
abi-dumper in particular wants the libraries it inspects in this form.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Juergen Gross <jgross@suse.com>
---
tools/Rules.mk | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/Rules.mk b/tools/Rules.mk
index f61da81f4a..2907ed2d39 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -106,8 +106,9 @@ endif
CFLAGS_libxenlight += $(CFLAGS_libxenctrl)
ifeq ($(debug),y)
-# Disable optimizations
-CFLAGS += -O0 -fno-omit-frame-pointer
+# Use -Og if available, -O0 otherwise
+dbg_opt_level := $(call cc-option,$(CC),-Og,-O0)
+CFLAGS += $(dbg_opt_level) -fno-omit-frame-pointer
# But allow an override to -O0 in case Python enforces -D_FORTIFY_SOURCE=<n>.
PY_CFLAGS += $(PY_NOOPT_CFLAGS)
else
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 07/10] tools: Use -Og for debug builds when available
2021-02-12 15:39 ` [PATCH 07/10] tools: Use -Og for debug builds when available Andrew Cooper
@ 2021-02-12 16:04 ` Jan Beulich
2021-02-12 16:09 ` Andrew Cooper
2021-02-16 16:26 ` Ian Jackson
2021-02-16 16:11 ` Ian Jackson
1 sibling, 2 replies; 47+ messages in thread
From: Jan Beulich @ 2021-02-12 16:04 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Ian Jackson, Wei Liu, Juergen Gross, Xen-devel
On 12.02.2021 16:39, Andrew Cooper wrote:
> The recommended optimisation level for debugging is -Og, and is what tools
> such as gdb prefer. In practice, it equates to -01 with a few specific
> optimisations turned off.
>
> abi-dumper in particular wants the libraries it inspects in this form.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -106,8 +106,9 @@ endif
> CFLAGS_libxenlight += $(CFLAGS_libxenctrl)
>
> ifeq ($(debug),y)
> -# Disable optimizations
> -CFLAGS += -O0 -fno-omit-frame-pointer
> +# Use -Og if available, -O0 otherwise
> +dbg_opt_level := $(call cc-option,$(CC),-Og,-O0)
> +CFLAGS += $(dbg_opt_level) -fno-omit-frame-pointer
I wonder if we shouldn't do something similar for the hypervisor,
where we use -O1 for debug builds right now. At least when
DEBUG_INFO is also enabled, -Og may be better.
Jan
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 07/10] tools: Use -Og for debug builds when available
2021-02-12 16:04 ` Jan Beulich
@ 2021-02-12 16:09 ` Andrew Cooper
2021-02-12 16:14 ` Jan Beulich
2021-02-16 16:26 ` Ian Jackson
1 sibling, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 16:09 UTC (permalink / raw)
To: xen-devel
On 12/02/2021 16:04, Jan Beulich wrote:
> On 12.02.2021 16:39, Andrew Cooper wrote:
>> The recommended optimisation level for debugging is -Og, and is what tools
>> such as gdb prefer. In practice, it equates to -01 with a few specific
>> optimisations turned off.
>>
>> abi-dumper in particular wants the libraries it inspects in this form.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Thanks,
>
>> --- a/tools/Rules.mk
>> +++ b/tools/Rules.mk
>> @@ -106,8 +106,9 @@ endif
>> CFLAGS_libxenlight += $(CFLAGS_libxenctrl)
>>
>> ifeq ($(debug),y)
>> -# Disable optimizations
>> -CFLAGS += -O0 -fno-omit-frame-pointer
>> +# Use -Og if available, -O0 otherwise
>> +dbg_opt_level := $(call cc-option,$(CC),-Og,-O0)
>> +CFLAGS += $(dbg_opt_level) -fno-omit-frame-pointer
> I wonder if we shouldn't do something similar for the hypervisor,
> where we use -O1 for debug builds right now. At least when
> DEBUG_INFO is also enabled, -Og may be better.
I also made that work... its rather more invasive in terms of changes -
all for "maybe uninitialised" warnings.
$ git diff e2bab84984^ --stat
xen/Makefile | 3 ++-
xen/arch/arm/domain_build.c | 2 +-
xen/arch/x86/irq.c | 2 +-
xen/arch/x86/mm/shadow/common.c | 2 +-
xen/arch/x86/pv/shim.c | 6 +++---
xen/arch/x86/sysctl.c | 4 ++--
xen/common/efi/boot.c | 2 +-
7 files changed, 11 insertions(+), 10 deletions(-)
is what is required to make Gitlab happy. I was planning to defer it to
4.16 at this point.
~Andrew
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 07/10] tools: Use -Og for debug builds when available
2021-02-12 16:09 ` Andrew Cooper
@ 2021-02-12 16:14 ` Jan Beulich
0 siblings, 0 replies; 47+ messages in thread
From: Jan Beulich @ 2021-02-12 16:14 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel
On 12.02.2021 17:09, Andrew Cooper wrote:
> On 12/02/2021 16:04, Jan Beulich wrote:
>> On 12.02.2021 16:39, Andrew Cooper wrote:
>>> --- a/tools/Rules.mk
>>> +++ b/tools/Rules.mk
>>> @@ -106,8 +106,9 @@ endif
>>> CFLAGS_libxenlight += $(CFLAGS_libxenctrl)
>>>
>>> ifeq ($(debug),y)
>>> -# Disable optimizations
>>> -CFLAGS += -O0 -fno-omit-frame-pointer
>>> +# Use -Og if available, -O0 otherwise
>>> +dbg_opt_level := $(call cc-option,$(CC),-Og,-O0)
>>> +CFLAGS += $(dbg_opt_level) -fno-omit-frame-pointer
>> I wonder if we shouldn't do something similar for the hypervisor,
>> where we use -O1 for debug builds right now. At least when
>> DEBUG_INFO is also enabled, -Og may be better.
>
> I also made that work... its rather more invasive in terms of changes -
> all for "maybe uninitialised" warnings.
>
> $ git diff e2bab84984^ --stat
> xen/Makefile | 3 ++-
> xen/arch/arm/domain_build.c | 2 +-
> xen/arch/x86/irq.c | 2 +-
> xen/arch/x86/mm/shadow/common.c | 2 +-
> xen/arch/x86/pv/shim.c | 6 +++---
> xen/arch/x86/sysctl.c | 4 ++--
> xen/common/efi/boot.c | 2 +-
> 7 files changed, 11 insertions(+), 10 deletions(-)
>
> is what is required to make Gitlab happy.
Oh, good to know. Thanks!
> I was planning to defer it to 4.16 at this point.
Of course.
Jan
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 07/10] tools: Use -Og for debug builds when available
2021-02-12 16:04 ` Jan Beulich
2021-02-12 16:09 ` Andrew Cooper
@ 2021-02-16 16:26 ` Ian Jackson
1 sibling, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 16:26 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, Juergen Gross, Xen-devel
Jan Beulich writes ("Re: [PATCH 07/10] tools: Use -Og for debug builds when available"):
> On 12.02.2021 16:39, Andrew Cooper wrote:
> > The recommended optimisation level for debugging is -Og, and is what tools
> > such as gdb prefer. In practice, it equates to -01 with a few specific
> > optimisations turned off.
> >
> > abi-dumper in particular wants the libraries it inspects in this form.
> >
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Jackson <iwj@xenproject.org>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 07/10] tools: Use -Og for debug builds when available
2021-02-12 15:39 ` [PATCH 07/10] tools: Use -Og for debug builds when available Andrew Cooper
2021-02-12 16:04 ` Jan Beulich
@ 2021-02-16 16:11 ` Ian Jackson
1 sibling, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 16:11 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Juergen Gross
Andrew Cooper writes ("[PATCH 07/10] tools: Use -Og for debug builds when available"):
> The recommended optimisation level for debugging is -Og, and is what tools
> such as gdb prefer. In practice, it equates to -01 with a few specific
> optimisations turned off.
>
> abi-dumper in particular wants the libraries it inspects in this form.
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
I would prefer to have this in 4.15 now than to backport it later...
Ian.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 08/10] tools: Check for abi-dumper in ./configure
2021-02-12 15:39 [PATCH for-4.15 00/10] tools: Support to use abi-dumper on libraries Andrew Cooper
` (6 preceding siblings ...)
2021-02-12 15:39 ` [PATCH 07/10] tools: Use -Og for debug builds when available Andrew Cooper
@ 2021-02-12 15:39 ` Andrew Cooper
2021-02-16 16:11 ` Ian Jackson
2021-02-16 16:27 ` Ian Jackson
2021-02-12 15:39 ` [PATCH 09/10] tools/libs: Add rule to generate headers.lst Andrew Cooper
2021-02-12 15:39 ` [PATCH 10/10] tools/libs: Write out an ABI analysis when abi-dumper is available Andrew Cooper
9 siblings, 2 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 15:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Juergen Gross
This will be optional. No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Juergen Gross <jgross@suse.com>
---
config/Tools.mk.in | 1 +
tools/configure | 41 +++++++++++++++++++++++++++++++++++++++++
tools/configure.ac | 1 +
3 files changed, 43 insertions(+)
diff --git a/config/Tools.mk.in b/config/Tools.mk.in
index 48bd9ab731..d47936686b 100644
--- a/config/Tools.mk.in
+++ b/config/Tools.mk.in
@@ -19,6 +19,7 @@ BCC := @BCC@
IASL := @IASL@
AWK := @AWK@
FETCHER := @FETCHER@
+ABI_DUMPER := @ABI_DUMPER@
# Extra folder for libs/includes
PREPEND_INCLUDES := @PREPEND_INCLUDES@
diff --git a/tools/configure b/tools/configure
index e63ca3797d..bb5acf9d43 100755
--- a/tools/configure
+++ b/tools/configure
@@ -682,6 +682,7 @@ OCAMLOPT
OCAMLLIB
OCAMLVERSION
OCAMLC
+ABI_DUMPER
INSTALL_DATA
INSTALL_SCRIPT
INSTALL_PROGRAM
@@ -5442,6 +5443,46 @@ $as_echo "no" >&6; }
fi
+# Extract the first word of "abi-dumper", so it can be a program name with args.
+set dummy abi-dumper; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_path_ABI_DUMPER+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ case $ABI_DUMPER in
+ [\\/]* | ?:[\\/]*)
+ ac_cv_path_ABI_DUMPER="$ABI_DUMPER" # Let the user override the test with a path.
+ ;;
+ *)
+ as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+ IFS=$as_save_IFS
+ test -z "$as_dir" && as_dir=.
+ for ac_exec_ext in '' $ac_executable_extensions; do
+ if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+ ac_cv_path_ABI_DUMPER="$as_dir/$ac_word$ac_exec_ext"
+ $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+ break 2
+ fi
+done
+ done
+IFS=$as_save_IFS
+
+ ;;
+esac
+fi
+ABI_DUMPER=$ac_cv_path_ABI_DUMPER
+if test -n "$ABI_DUMPER"; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ABI_DUMPER" >&5
+$as_echo "$ABI_DUMPER" >&6; }
+else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
# Extract the first word of "perl", so it can be a program name with args.
set dummy perl; ac_word=$2
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
diff --git a/tools/configure.ac b/tools/configure.ac
index 6b611deb13..636e7077be 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -310,6 +310,7 @@ AC_PROG_CC
AC_PROG_MAKE_SET
AC_PROG_INSTALL
AC_PATH_PROG([FLEX], [flex])
+AC_PATH_PROG([ABI_DUMPER], [abi-dumper])
AX_PATH_PROG_OR_FAIL([PERL], [perl])
AX_PATH_PROG_OR_FAIL([AWK], [awk])
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH 09/10] tools/libs: Add rule to generate headers.lst
2021-02-12 15:39 [PATCH for-4.15 00/10] tools: Support to use abi-dumper on libraries Andrew Cooper
` (7 preceding siblings ...)
2021-02-12 15:39 ` [PATCH 08/10] tools: Check for abi-dumper in ./configure Andrew Cooper
@ 2021-02-12 15:39 ` Andrew Cooper
2021-02-16 16:13 ` Ian Jackson
2021-02-16 16:48 ` [PATCH v2 " Andrew Cooper
2021-02-12 15:39 ` [PATCH 10/10] tools/libs: Write out an ABI analysis when abi-dumper is available Andrew Cooper
9 siblings, 2 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 15:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Juergen Gross
abi-dumper needs a list of the public header files for shared objects, and
only accepts this in the form of a file.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Juergen Gross <jgross@suse.com>
---
tools/libs/.gitignore | 1 +
tools/libs/libs.mk | 9 ++++++++-
2 files changed, 9 insertions(+), 1 deletion(-)
create mode 100644 tools/libs/.gitignore
diff --git a/tools/libs/.gitignore b/tools/libs/.gitignore
new file mode 100644
index 0000000000..4a13126144
--- /dev/null
+++ b/tools/libs/.gitignore
@@ -0,0 +1 @@
+*/headers.lst
diff --git a/tools/libs/libs.mk b/tools/libs/libs.mk
index 0b3381755a..ac68996ab2 100644
--- a/tools/libs/libs.mk
+++ b/tools/libs/libs.mk
@@ -76,6 +76,10 @@ endif
headers.chk: $(AUTOINCS)
+headers.lst: FORCE
+ @{ $(foreach h,$(LIBHEADERS),echo $(h);) } > $@.tmp
+ @$(call move-if-changed,$@.tmp,$@)
+
libxen$(LIBNAME).map:
echo 'VERS_$(MAJOR).$(MINOR) { global: *; };' >$@
@@ -118,9 +122,12 @@ TAGS:
clean:
rm -rf *.rpm $(LIB) *~ $(DEPS_RM) $(LIB_OBJS) $(PIC_OBJS)
rm -f lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR) lib$(LIB_FILE_NAME).so.$(MAJOR)
- rm -f headers.chk
+ rm -f headers.chk headers.lst
rm -f $(PKG_CONFIG)
rm -f _paths.h
.PHONY: distclean
distclean: clean
+
+.PHONY: FORCE
+FORCE:
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 09/10] tools/libs: Add rule to generate headers.lst
2021-02-12 15:39 ` [PATCH 09/10] tools/libs: Add rule to generate headers.lst Andrew Cooper
@ 2021-02-16 16:13 ` Ian Jackson
2021-02-16 16:48 ` [PATCH v2 " Andrew Cooper
1 sibling, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 16:13 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Juergen Gross
Andrew Cooper writes ("[PATCH 09/10] tools/libs: Add rule to generate headers.lst"):
> abi-dumper needs a list of the public header files for shared objects, and
> only accepts this in the form of a file.
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
because it's not run by default, but...
> +headers.lst: FORCE
> + @{ $(foreach h,$(LIBHEADERS),echo $(h);) } > $@.tmp
Missing set -e. If the disk fills up temporarily you might get a
partial file here...
> + @$(call move-if-changed,$@.tmp,$@)
Ian.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v2 09/10] tools/libs: Add rule to generate headers.lst
2021-02-12 15:39 ` [PATCH 09/10] tools/libs: Add rule to generate headers.lst Andrew Cooper
2021-02-16 16:13 ` Ian Jackson
@ 2021-02-16 16:48 ` Andrew Cooper
2021-02-16 17:32 ` Ian Jackson
1 sibling, 1 reply; 47+ messages in thread
From: Andrew Cooper @ 2021-02-16 16:48 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Juergen Gross
abi-dumper needs a list of the public header files for shared objects, and
only accepts this in the form of a file.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Juergen Gross <jgross@suse.com>
v2:
* Use set -e to avoid truncated content on transient errors
---
tools/libs/.gitignore | 1 +
tools/libs/libs.mk | 9 ++++++++-
2 files changed, 9 insertions(+), 1 deletion(-)
create mode 100644 tools/libs/.gitignore
diff --git a/tools/libs/.gitignore b/tools/libs/.gitignore
new file mode 100644
index 0000000000..4a13126144
--- /dev/null
+++ b/tools/libs/.gitignore
@@ -0,0 +1 @@
+*/headers.lst
diff --git a/tools/libs/libs.mk b/tools/libs/libs.mk
index 0b3381755a..7970627ac8 100644
--- a/tools/libs/libs.mk
+++ b/tools/libs/libs.mk
@@ -76,6 +76,10 @@ endif
headers.chk: $(AUTOINCS)
+headers.lst: FORCE
+ @{ set -e; $(foreach h,$(LIBHEADERS),echo $(h);) } > $@.tmp
+ @$(call move-if-changed,$@.tmp,$@)
+
libxen$(LIBNAME).map:
echo 'VERS_$(MAJOR).$(MINOR) { global: *; };' >$@
@@ -118,9 +122,12 @@ TAGS:
clean:
rm -rf *.rpm $(LIB) *~ $(DEPS_RM) $(LIB_OBJS) $(PIC_OBJS)
rm -f lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR) lib$(LIB_FILE_NAME).so.$(MAJOR)
- rm -f headers.chk
+ rm -f headers.chk headers.lst
rm -f $(PKG_CONFIG)
rm -f _paths.h
.PHONY: distclean
distclean: clean
+
+.PHONY: FORCE
+FORCE:
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v2 09/10] tools/libs: Add rule to generate headers.lst
2021-02-16 16:48 ` [PATCH v2 " Andrew Cooper
@ 2021-02-16 17:32 ` Ian Jackson
0 siblings, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 17:32 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Juergen Gross
Andrew Cooper writes ("[PATCH v2 09/10] tools/libs: Add rule to generate headers.lst"):
> abi-dumper needs a list of the public header files for shared objects, and
> only accepts this in the form of a file.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Release-Acked-by: Ian Jackson <iwj@xenproject.org>
Reviewed-by: Ian Jackson <iwj@xenproject.org>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 10/10] tools/libs: Write out an ABI analysis when abi-dumper is available
2021-02-12 15:39 [PATCH for-4.15 00/10] tools: Support to use abi-dumper on libraries Andrew Cooper
` (8 preceding siblings ...)
2021-02-12 15:39 ` [PATCH 09/10] tools/libs: Add rule to generate headers.lst Andrew Cooper
@ 2021-02-12 15:39 ` Andrew Cooper
2021-02-12 16:12 ` Jan Beulich
` (3 more replies)
9 siblings, 4 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 15:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Juergen Gross
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Juergen Gross <jgross@suse.com>
---
tools/libs/libs.mk | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tools/libs/libs.mk b/tools/libs/libs.mk
index ac68996ab2..2a4ce8a90c 100644
--- a/tools/libs/libs.mk
+++ b/tools/libs/libs.mk
@@ -49,6 +49,8 @@ PKG_CONFIG_LOCAL := $(PKG_CONFIG_DIR)/$(PKG_CONFIG)
LIBHEADER ?= $(LIB_FILE_NAME).h
LIBHEADERS = $(foreach h, $(LIBHEADER), $(XEN_INCLUDE)/$(h))
+PKG_ABI := lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR)-$(XEN_COMPILE_ARCH)-abi.dump
+
$(PKG_CONFIG_LOCAL): PKG_CONFIG_PREFIX = $(XEN_ROOT)
$(PKG_CONFIG_LOCAL): PKG_CONFIG_INCDIR = $(XEN_INCLUDE)
$(PKG_CONFIG_LOCAL): PKG_CONFIG_LIBDIR = $(CURDIR)
@@ -94,6 +96,13 @@ lib$(LIB_FILE_NAME).so.$(MAJOR): lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR)
lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR): $(PIC_OBJS) libxen$(LIBNAME).map
$(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,lib$(LIB_FILE_NAME).so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $(PIC_OBJS) $(LDUSELIBS) $(APPEND_LDFLAGS)
+# If abi-dumper is available, write out the ABI analysis
+ifneq ($(ABI_DUMPER),)
+libs: $(PKG_ABI)
+$(PKG_ABI): lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR) headers.lst
+ abi-dumper $< -o $@ -public-headers headers.lst -lver $(MAJOR).$(MINOR)
+endif
+
.PHONY: install
install: build
$(INSTALL_DIR) $(DESTDIR)$(libdir)
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 10/10] tools/libs: Write out an ABI analysis when abi-dumper is available
2021-02-12 15:39 ` [PATCH 10/10] tools/libs: Write out an ABI analysis when abi-dumper is available Andrew Cooper
@ 2021-02-12 16:12 ` Jan Beulich
2021-02-12 17:03 ` Andrew Cooper
2021-02-12 18:01 ` [PATCH v1.1 " Andrew Cooper
` (2 subsequent siblings)
3 siblings, 1 reply; 47+ messages in thread
From: Jan Beulich @ 2021-02-12 16:12 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Ian Jackson, Wei Liu, Juergen Gross, Xen-devel
On 12.02.2021 16:39, Andrew Cooper wrote:
> --- a/tools/libs/libs.mk
> +++ b/tools/libs/libs.mk
> @@ -49,6 +49,8 @@ PKG_CONFIG_LOCAL := $(PKG_CONFIG_DIR)/$(PKG_CONFIG)
> LIBHEADER ?= $(LIB_FILE_NAME).h
> LIBHEADERS = $(foreach h, $(LIBHEADER), $(XEN_INCLUDE)/$(h))
>
> +PKG_ABI := lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR)-$(XEN_COMPILE_ARCH)-abi.dump
Don't you mean $(XEN_TARGET_ARCH) here?
Jan
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 10/10] tools/libs: Write out an ABI analysis when abi-dumper is available
2021-02-12 16:12 ` Jan Beulich
@ 2021-02-12 17:03 ` Andrew Cooper
0 siblings, 0 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 17:03 UTC (permalink / raw)
To: Jan Beulich; +Cc: Ian Jackson, Wei Liu, Juergen Gross, Xen-devel
On 12/02/2021 16:12, Jan Beulich wrote:
> On 12.02.2021 16:39, Andrew Cooper wrote:
>> --- a/tools/libs/libs.mk
>> +++ b/tools/libs/libs.mk
>> @@ -49,6 +49,8 @@ PKG_CONFIG_LOCAL := $(PKG_CONFIG_DIR)/$(PKG_CONFIG)
>> LIBHEADER ?= $(LIB_FILE_NAME).h
>> LIBHEADERS = $(foreach h, $(LIBHEADER), $(XEN_INCLUDE)/$(h))
>>
>> +PKG_ABI := lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR)-$(XEN_COMPILE_ARCH)-abi.dump
> Don't you mean $(XEN_TARGET_ARCH) here?
Yes, I do. Will fix up.
Thanks,
~Andrew
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v1.1 10/10] tools/libs: Write out an ABI analysis when abi-dumper is available
2021-02-12 15:39 ` [PATCH 10/10] tools/libs: Write out an ABI analysis when abi-dumper is available Andrew Cooper
2021-02-12 16:12 ` Jan Beulich
@ 2021-02-12 18:01 ` Andrew Cooper
2021-02-16 16:15 ` [PATCH " Ian Jackson
2021-02-16 16:30 ` Ian Jackson
3 siblings, 0 replies; 47+ messages in thread
From: Andrew Cooper @ 2021-02-12 18:01 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Juergen Gross
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Juergen Gross <jgross@suse.com>
v2:
* Swap XEN_COMPILE_ARCH for XEN_TARGET_ARCH
* Swap abi-dumper for $(ABI_DUMPER)
---
tools/libs/libs.mk | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tools/libs/libs.mk b/tools/libs/libs.mk
index ac68996ab2..a82d1783cc 100644
--- a/tools/libs/libs.mk
+++ b/tools/libs/libs.mk
@@ -49,6 +49,8 @@ PKG_CONFIG_LOCAL := $(PKG_CONFIG_DIR)/$(PKG_CONFIG)
LIBHEADER ?= $(LIB_FILE_NAME).h
LIBHEADERS = $(foreach h, $(LIBHEADER), $(XEN_INCLUDE)/$(h))
+PKG_ABI := lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR)-$(XEN_TARGET_ARCH)-abi.dump
+
$(PKG_CONFIG_LOCAL): PKG_CONFIG_PREFIX = $(XEN_ROOT)
$(PKG_CONFIG_LOCAL): PKG_CONFIG_INCDIR = $(XEN_INCLUDE)
$(PKG_CONFIG_LOCAL): PKG_CONFIG_LIBDIR = $(CURDIR)
@@ -94,6 +96,13 @@ lib$(LIB_FILE_NAME).so.$(MAJOR): lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR)
lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR): $(PIC_OBJS) libxen$(LIBNAME).map
$(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,lib$(LIB_FILE_NAME).so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $(PIC_OBJS) $(LDUSELIBS) $(APPEND_LDFLAGS)
+# If abi-dumper is available, write out the ABI analysis
+ifneq ($(ABI_DUMPER),)
+libs: $(PKG_ABI)
+$(PKG_ABI): lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR) headers.lst
+ $(ABI_DUMPER) $< -o $@ -public-headers headers.lst -lver $(MAJOR).$(MINOR)
+endif
+
.PHONY: install
install: build
$(INSTALL_DIR) $(DESTDIR)$(libdir)
--
2.11.0
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 10/10] tools/libs: Write out an ABI analysis when abi-dumper is available
2021-02-12 15:39 ` [PATCH 10/10] tools/libs: Write out an ABI analysis when abi-dumper is available Andrew Cooper
2021-02-12 16:12 ` Jan Beulich
2021-02-12 18:01 ` [PATCH v1.1 " Andrew Cooper
@ 2021-02-16 16:15 ` Ian Jackson
2021-02-16 16:30 ` Ian Jackson
3 siblings, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 16:15 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Juergen Gross
Andrew Cooper writes ("[PATCH 10/10] tools/libs: Write out an ABI analysis when abi-dumper is available"):
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 10/10] tools/libs: Write out an ABI analysis when abi-dumper is available
2021-02-12 15:39 ` [PATCH 10/10] tools/libs: Write out an ABI analysis when abi-dumper is available Andrew Cooper
` (2 preceding siblings ...)
2021-02-16 16:15 ` [PATCH " Ian Jackson
@ 2021-02-16 16:30 ` Ian Jackson
3 siblings, 0 replies; 47+ messages in thread
From: Ian Jackson @ 2021-02-16 16:30 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Juergen Gross
Andrew Cooper writes ("[PATCH 10/10] tools/libs: Write out an ABI analysis when abi-dumper is available"):
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
...
> +# If abi-dumper is available, write out the ABI analysis
> +ifneq ($(ABI_DUMPER),)
> +libs: $(PKG_ABI)
> +$(PKG_ABI): lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR) headers.lst
> + abi-dumper $< -o $@ -public-headers headers.lst -lver $(MAJOR).$(MINOR)
> +endif
Kind of annoying that we don't have a variable for
lib$(LIB_FILE_NAME).so.$(MAJOR).$(MINOR)
Reviewed-by: Ian Jackson <iwj@xenproject.org>
^ permalink raw reply [flat|nested] 47+ messages in thread