From 3e3138b4a9565934487d7b39019282e75a894487 Mon Sep 17 00:00:00 2001 From: Patrick Oppenlander Date: Thu, 9 Jul 2020 14:14:51 +1000 Subject: [PATCH 001/104] libfdt: fix fdt_check_full buffer overrun fdt_check_header assumes that its argument points to a complete header and can read data beyond the FDT_V1_SIZE bytes which fdt_check_full can provide. fdt_header_size can safely return a header size with FDT_V1_SIZE bytes available and will return a usable value even for a corrupted header. Signed-off-by: Patrick Oppenlander Message-Id: <20200709041451.338548-1-patrick.oppenlander@gmail.com> Signed-off-by: David Gibson --- libfdt/fdt_check.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libfdt/fdt_check.c b/libfdt/fdt_check.c index 7f6a96c..9ddfdbf 100644 --- a/libfdt/fdt_check.c +++ b/libfdt/fdt_check.c @@ -22,6 +22,8 @@ int fdt_check_full(const void *fdt, size_t bufsize) if (bufsize < FDT_V1_SIZE) return -FDT_ERR_TRUNCATED; + if (bufsize < fdt_header_size(fdt)) + return -FDT_ERR_TRUNCATED; err = fdt_check_header(fdt); if (err != 0) return err; From 808cdaaf524f6beb95e0ea87fc42711dbddef21a Mon Sep 17 00:00:00 2001 From: Andrei Ziureaev Date: Tue, 14 Jul 2020 16:45:37 +0100 Subject: [PATCH 002/104] dtc: Avoid UB when shifting Prevent undefined behavior when shifting by a number that's bigger than or equal to the width of the first operand. Signed-off-by: Andrei Ziureaev Message-Id: <20200714154542.18064-2-andrei.ziureaev@arm.com> Signed-off-by: David Gibson --- dtc-parser.y | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dtc-parser.y b/dtc-parser.y index 40dcf4f..a0316a3 100644 --- a/dtc-parser.y +++ b/dtc-parser.y @@ -476,8 +476,8 @@ integer_rela: ; integer_shift: - integer_shift DT_LSHIFT integer_add { $$ = $1 << $3; } - | integer_shift DT_RSHIFT integer_add { $$ = $1 >> $3; } + integer_shift DT_LSHIFT integer_add { $$ = ($3 < 64) ? ($1 << $3) : 0; } + | integer_shift DT_RSHIFT integer_add { $$ = ($3 < 64) ? ($1 >> $3) : 0; } | integer_add ; From 3d522abc7571e1851ce71fcf0c3452ef287b1bab Mon Sep 17 00:00:00 2001 From: Andrei Ziureaev Date: Tue, 21 Jul 2020 16:58:57 +0100 Subject: [PATCH 003/104] dtc: Include stdlib.h in util.h If used on its own, util.h needs stdlib.h for exit(), malloc() and realloc(). Signed-off-by: Andrei Ziureaev Message-Id: <20200721155900.9147-2-andrei.ziureaev@arm.com> Signed-off-by: David Gibson --- util.h | 1 + 1 file changed, 1 insertion(+) diff --git a/util.h b/util.h index 5a4172d..a771b46 100644 --- a/util.h +++ b/util.h @@ -2,6 +2,7 @@ #ifndef UTIL_H #define UTIL_H +#include #include #include #include From 7bb86f1c09563f502c6ab0166feacf346e4e8c40 Mon Sep 17 00:00:00 2001 From: Frank Mehnert Date: Thu, 13 Aug 2020 17:26:26 +0200 Subject: [PATCH 004/104] libfdt: fix fdt_check_node_offset_ w/ VALID_INPUT fdt_check_node_offset_() checks for a valid offset but also changes the offset by calling fdt_next_tag(). Hence, do not skip this function if ASSUME_VALID_INPUT is set but only omit the initial offset check in that case. As this function works very similar to fdt_check_prop_offset_(), do the offset check there as well depending on ASSUME_VALID_INPUT. Message-Id: <1913141.TlUzK5foHS@noys4> Signed-off-by: David Gibson --- libfdt/fdt.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/libfdt/fdt.c b/libfdt/fdt.c index c28fcc1..37b7b93 100644 --- a/libfdt/fdt.c +++ b/libfdt/fdt.c @@ -206,10 +206,11 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset) int fdt_check_node_offset_(const void *fdt, int offset) { - if (can_assume(VALID_INPUT)) - return offset; - if ((offset < 0) || (offset % FDT_TAGSIZE) - || (fdt_next_tag(fdt, offset, &offset) != FDT_BEGIN_NODE)) + if (!can_assume(VALID_INPUT) + && ((offset < 0) || (offset % FDT_TAGSIZE))) + return -FDT_ERR_BADOFFSET; + + if (fdt_next_tag(fdt, offset, &offset) != FDT_BEGIN_NODE) return -FDT_ERR_BADOFFSET; return offset; @@ -217,8 +218,11 @@ int fdt_check_node_offset_(const void *fdt, int offset) int fdt_check_prop_offset_(const void *fdt, int offset) { - if ((offset < 0) || (offset % FDT_TAGSIZE) - || (fdt_next_tag(fdt, offset, &offset) != FDT_PROP)) + if (!can_assume(VALID_INPUT) + && ((offset < 0) || (offset % FDT_TAGSIZE))) + return -FDT_ERR_BADOFFSET; + + if (fdt_next_tag(fdt, offset, &offset) != FDT_PROP) return -FDT_ERR_BADOFFSET; return offset; From ca19c3db2bf62000101ae8f83c37cd6e0d44d218 Mon Sep 17 00:00:00 2001 From: Emmanuel Vadot Date: Tue, 25 Aug 2020 12:34:18 +0200 Subject: [PATCH 005/104] Makefile: Specify cflags for libyaml Some systems don't install third party software includes in a default path (like FreeBSD), add yaml cflags to fix compilation. Signed-off-by: Emmanuel Vadot --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index cb256e8..c187d5f 100644 --- a/Makefile +++ b/Makefile @@ -59,6 +59,7 @@ ifeq ($(NO_YAML),1) CFLAGS += -DNO_YAML else LDLIBS_dtc += $(shell $(PKG_CONFIG) --libs yaml-0.1) + CFLAGS += $(shell $(PKG_CONFIG) --cflags yaml-0.1) endif ifeq ($(HOSTOS),darwin) From 442ea3dd157906ce23df8001095562d3492a471c Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 21 Sep 2020 17:52:50 +0100 Subject: [PATCH 006/104] libfdt: fdt_offset_ptr(): Fix comparison warnings With -Wsign-compare, compilers warn about mismatching signedness in comparisons in fdt_offset_ptr(). This mostly stems from "offset" being passed in as a signed integer, even though the function would not really tolerate negative values. Short of changing the prototype, check that offset is not negative, and use an unsigned type internally. Signed-off-by: Andre Przywara Message-Id: <20200921165303.9115-2-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/fdt.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/libfdt/fdt.c b/libfdt/fdt.c index 37b7b93..04e1e06 100644 --- a/libfdt/fdt.c +++ b/libfdt/fdt.c @@ -134,16 +134,20 @@ int fdt_check_header(const void *fdt) const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len) { - unsigned absoffset = offset + fdt_off_dt_struct(fdt); + unsigned int uoffset = offset; + unsigned int absoffset = offset + fdt_off_dt_struct(fdt); + + if (offset < 0) + return NULL; if (!can_assume(VALID_INPUT)) - if ((absoffset < offset) + if ((absoffset < uoffset) || ((absoffset + len) < absoffset) || (absoffset + len) > fdt_totalsize(fdt)) return NULL; if (can_assume(LATEST) || fdt_version(fdt) >= 0x11) - if (((offset + len) < offset) + if (((uoffset + len) < uoffset) || ((offset + len) > fdt_size_dt_struct(fdt))) return NULL; From 0c43d4d7bf5a3a2e83d2005daf1e44ea5bd7f069 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 21 Sep 2020 17:52:51 +0100 Subject: [PATCH 007/104] libfdt: fdt_mem_rsv(): Fix comparison warnings With -Wsign-compare, compilers warn about a mismatching signedness in comparisons in fdt_mem_rsv(). Since all involved values must be positive, change the used types to be unsigned. Signed-off-by: Andre Przywara Message-Id: <20200921165303.9115-3-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/fdt_ro.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c index e03570a..3c8d143 100644 --- a/libfdt/fdt_ro.c +++ b/libfdt/fdt_ro.c @@ -157,8 +157,8 @@ int fdt_generate_phandle(const void *fdt, uint32_t *phandle) static const struct fdt_reserve_entry *fdt_mem_rsv(const void *fdt, int n) { - int offset = n * sizeof(struct fdt_reserve_entry); - int absoffset = fdt_off_mem_rsvmap(fdt) + offset; + unsigned int offset = n * sizeof(struct fdt_reserve_entry); + unsigned int absoffset = fdt_off_mem_rsvmap(fdt) + offset; if (!can_assume(VALID_INPUT)) { if (absoffset < fdt_off_mem_rsvmap(fdt)) From f8e11e61624e7ea9307655fe4b5bb66f2d0c1be8 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 21 Sep 2020 17:52:52 +0100 Subject: [PATCH 008/104] libfdt: fdt_grab_space_(): Fix comparison warning With -Wsign-compare, compilers warn about a mismatching signedness in a comparison in fdt_grab_space_(). All the involved values cannot be negative, so let's switch the types of the local variables to unsigned to make the compiler happy. Signed-off-by: Andre Przywara Message-Id: <20200921165303.9115-4-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/fdt_sw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libfdt/fdt_sw.c b/libfdt/fdt_sw.c index 94ce4bb..d10a720 100644 --- a/libfdt/fdt_sw.c +++ b/libfdt/fdt_sw.c @@ -93,8 +93,8 @@ static inline uint32_t sw_flags(void *fdt) static void *fdt_grab_space_(void *fdt, size_t len) { - int offset = fdt_size_dt_struct(fdt); - int spaceleft; + unsigned int offset = fdt_size_dt_struct(fdt); + unsigned int spaceleft; spaceleft = fdt_totalsize(fdt) - fdt_off_dt_struct(fdt) - fdt_size_dt_strings(fdt); From 54dca098531699dc8beeaf97e3525b9b5eb23b0a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 21 Sep 2020 17:52:55 +0100 Subject: [PATCH 009/104] libfdt: fdt_get_string(): Fix comparison warnings With -Wsign-compare, compilers warn about a mismatching signedness in comparisons in fdt_get_string(). In the first two cases, we have just established that the signed values are not negative, so it's safe to cast the values to an unsigned type. Signed-off-by: Simon Glass Signed-off-by: Andre Przywara Message-Id: <20200921165303.9115-7-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/fdt_ro.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c index 3c8d143..ddb5a2d 100644 --- a/libfdt/fdt_ro.c +++ b/libfdt/fdt_ro.c @@ -53,7 +53,7 @@ const char *fdt_get_string(const void *fdt, int stroffset, int *lenp) err = -FDT_ERR_BADOFFSET; absoffset = stroffset + fdt_off_dt_strings(fdt); - if (absoffset >= totalsize) + if (absoffset >= (unsigned)totalsize) goto fail; len = totalsize - absoffset; @@ -61,7 +61,7 @@ const char *fdt_get_string(const void *fdt, int stroffset, int *lenp) if (stroffset < 0) goto fail; if (can_assume(LATEST) || fdt_version(fdt) >= 17) { - if (stroffset >= fdt_size_dt_strings(fdt)) + if ((unsigned)stroffset >= fdt_size_dt_strings(fdt)) goto fail; if ((fdt_size_dt_strings(fdt) - stroffset) < len) len = fdt_size_dt_strings(fdt) - stroffset; From faa76fc10bc5819899d6bc78535d212acaccbd6a Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 21 Sep 2020 17:52:56 +0100 Subject: [PATCH 010/104] libfdt: fdt_splice_(): Fix comparison warning With -Wsign-compare, compilers warn about a mismatching signedness in a comparison in fdt_splice_(). Since we just established that oldlen is not negative, we can safely cast it to an unsigned type. Signed-off-by: Andre Przywara Message-Id: <20200921165303.9115-8-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/fdt_rw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c index 93e4a2b..68887b9 100644 --- a/libfdt/fdt_rw.c +++ b/libfdt/fdt_rw.c @@ -59,7 +59,7 @@ static int fdt_splice_(void *fdt, void *splicepoint, int oldlen, int newlen) if ((oldlen < 0) || (soff + oldlen < soff) || (soff + oldlen > dsize)) return -FDT_ERR_BADOFFSET; - if ((p < (char *)fdt) || (dsize + newlen < oldlen)) + if ((p < (char *)fdt) || (dsize + newlen < (unsigned)oldlen)) return -FDT_ERR_BADOFFSET; if (dsize - oldlen + newlen > fdt_totalsize(fdt)) return -FDT_ERR_NOSPACE; From ce9e1f25a7de457734d0fb314224c99102e7ebbe Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 21 Sep 2020 17:52:58 +0100 Subject: [PATCH 011/104] libfdt: fdt_resize(): Fix comparison warning With -Wsign-compare, compilers warn about a mismatching signedness in a comparison in fdt_resize(). A negative buffer size will surely do us no good, so let's rule this case out first. In the actual comparison we then know that a cast to an unsigned type is safe. Signed-off-by: Andre Przywara Message-Id: <20200921165303.9115-10-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/fdt_sw.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libfdt/fdt_sw.c b/libfdt/fdt_sw.c index d10a720..8de18fd 100644 --- a/libfdt/fdt_sw.c +++ b/libfdt/fdt_sw.c @@ -152,6 +152,9 @@ int fdt_resize(void *fdt, void *buf, int bufsize) FDT_SW_PROBE(fdt); + if (bufsize < 0) + return -FDT_ERR_NOSPACE; + headsize = fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt); tailsize = fdt_size_dt_strings(fdt); @@ -159,7 +162,7 @@ int fdt_resize(void *fdt, void *buf, int bufsize) headsize + tailsize > fdt_totalsize(fdt)) return -FDT_ERR_INTERNAL; - if ((headsize + tailsize) > bufsize) + if ((headsize + tailsize) > (unsigned)bufsize) return -FDT_ERR_NOSPACE; oldtail = (char *)fdt + fdt_totalsize(fdt) - tailsize; From 07158f4cf2a2fa2267b7a86f138a4f16ecb54172 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 21 Sep 2020 17:53:00 +0100 Subject: [PATCH 012/104] libfdt: overlay: Fix comparison warning With -Wsign-compare, compilers warn about a mismatching signedness in a comparison in overlay_update_local_node_references(). This happens because the division of a signed int by an unsigned int promotes the dividend to unsigned first (ANSI C standard 6.1.3.8). As in this case we basically just divide by 4, we can do the division separately earlier, which preserves the original type. Signed-off-by: Andre Przywara Message-Id: <20200921165303.9115-12-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/fdt_overlay.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c index b310e49..d217e79 100644 --- a/libfdt/fdt_overlay.c +++ b/libfdt/fdt_overlay.c @@ -241,6 +241,7 @@ static int overlay_update_local_node_references(void *fdto, if (fixup_len % sizeof(uint32_t)) return -FDT_ERR_BADOVERLAY; + fixup_len /= sizeof(uint32_t); tree_val = fdt_getprop(fdto, tree_node, name, &tree_len); if (!tree_val) { @@ -250,7 +251,7 @@ static int overlay_update_local_node_references(void *fdto, return tree_len; } - for (i = 0; i < (fixup_len / sizeof(uint32_t)); i++) { + for (i = 0; i < fixup_len; i++) { fdt32_t adj_val; uint32_t poffset; From 10f682788c3023d319cdc14f59ffe9d91cb6d2fc Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 21 Sep 2020 17:53:02 +0100 Subject: [PATCH 013/104] libfdt: fdt_node_offset_by_phandle(): Fix comparison warning With -Wsign-compare, compilers warn about a mismatching signedness in a comparison in fdt_node_offset_by_phandle(). Uses a better suited bitwise NOT operator to denote the special value of -1, which automatically results in an unsigned type. Signed-off-by: Andre Przywara Message-Id: <20200921165303.9115-14-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/fdt_ro.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c index ddb5a2d..e192184 100644 --- a/libfdt/fdt_ro.c +++ b/libfdt/fdt_ro.c @@ -680,7 +680,7 @@ int fdt_node_offset_by_phandle(const void *fdt, uint32_t phandle) { int offset; - if ((phandle == 0) || (phandle == -1)) + if ((phandle == 0) || (phandle == ~0U)) return -FDT_ERR_BADPHANDLE; FDT_RO_PROBE(fdt); From 3d7c6f44195a78dfa52b1b8c9efd7efd706b0dd9 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Thu, 1 Oct 2020 17:46:25 +0100 Subject: [PATCH 014/104] libfdt: fdt_add_string_(): Fix comparison warning With -Wsign-compare, compilers warn about a mismatching signedness in a comparison in fdt_add_string_(). Make all variables unsigned, and express the negative offset trick via subtractions in the code. Signed-off-by: Andre Przywara Message-Id: <20201001164630.4980-2-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/fdt_sw.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libfdt/fdt_sw.c b/libfdt/fdt_sw.c index 8de18fd..354f466 100644 --- a/libfdt/fdt_sw.c +++ b/libfdt/fdt_sw.c @@ -250,18 +250,18 @@ int fdt_end_node(void *fdt) static int fdt_add_string_(void *fdt, const char *s) { char *strtab = (char *)fdt + fdt_totalsize(fdt); - int strtabsize = fdt_size_dt_strings(fdt); - int len = strlen(s) + 1; - int struct_top, offset; + unsigned int strtabsize = fdt_size_dt_strings(fdt); + unsigned int len = strlen(s) + 1; + unsigned int struct_top, offset; - offset = -strtabsize - len; + offset = strtabsize + len; struct_top = fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt); - if (fdt_totalsize(fdt) + offset < struct_top) + if (fdt_totalsize(fdt) - offset < struct_top) return 0; /* no more room :( */ - memcpy(strtab + offset, s, len); + memcpy(strtab - offset, s, len); fdt_set_size_dt_strings(fdt, strtabsize + len); - return offset; + return -offset; } /* Must only be used to roll back in case of error */ From f28aa271000bfeaa765b824a8a3e7b170da12925 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Thu, 1 Oct 2020 17:46:26 +0100 Subject: [PATCH 015/104] libfdt: fdt_move(): Fix comparison warnings With -Wsign-compare, compilers warn about a mismatching signedness in comparisons in fdt_move(). This stems from "bufsize" being passed in as a signed integer, even though we would expect a buffer size to be positive. Short of changing the prototype, check that bufsize is not negative, and cast it to an unsigned type in the comparison. Signed-off-by: Andre Przywara Message-Id: <20201001164630.4980-3-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/fdt.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libfdt/fdt.c b/libfdt/fdt.c index 04e1e06..6cf2fa0 100644 --- a/libfdt/fdt.c +++ b/libfdt/fdt.c @@ -314,9 +314,12 @@ const char *fdt_find_string_(const char *strtab, int tabsize, const char *s) int fdt_move(const void *fdt, void *buf, int bufsize) { + if (!can_assume(VALID_INPUT) && bufsize < 0) + return -FDT_ERR_NOSPACE; + FDT_RO_PROBE(fdt); - if (fdt_totalsize(fdt) > bufsize) + if (fdt_totalsize(fdt) > (unsigned int)bufsize) return -FDT_ERR_NOSPACE; memmove(buf, fdt, fdt_totalsize(fdt)); From fb1f65f158327895acd360683e5c6da56d6944ea Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Thu, 1 Oct 2020 17:46:27 +0100 Subject: [PATCH 016/104] libfdt: fdt_create_with_flags(): Fix comparison warning With -Wsign-compare, compilers warn about a mismatching signedness in a comparison in fdt_create_with_flags(). By making hdrsize a signed integer (we are sure it's a very small number), we avoid all the casts and have matching types. Signed-off-by: Andre Przywara Message-Id: <20201001164630.4980-4-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/fdt_sw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libfdt/fdt_sw.c b/libfdt/fdt_sw.c index 354f466..68b543c 100644 --- a/libfdt/fdt_sw.c +++ b/libfdt/fdt_sw.c @@ -108,8 +108,8 @@ static void *fdt_grab_space_(void *fdt, size_t len) int fdt_create_with_flags(void *buf, int bufsize, uint32_t flags) { - const size_t hdrsize = FDT_ALIGN(sizeof(struct fdt_header), - sizeof(struct fdt_reserve_entry)); + const int hdrsize = FDT_ALIGN(sizeof(struct fdt_header), + sizeof(struct fdt_reserve_entry)); void *fdt = buf; if (bufsize < hdrsize) From 82525f41d59ec57a4e9325796a6f1baefc036236 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Thu, 1 Oct 2020 17:46:28 +0100 Subject: [PATCH 017/104] libfdt: libfdt_wip: Fix comparison warning With -Wsign-compare, compilers warn about a mismatching signedness in a comparison in fdt_setprop_inplace_namelen_partial(). fdt_getprop_namelen() will only return negative error values in "proplen" if the return value is NULL. So we can rely on "proplen" being positive in our case and can safely cast it to an unsigned type. Signed-off-by: Andre Przywara Message-Id: <20201001164630.4980-5-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/fdt_wip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfdt/fdt_wip.c b/libfdt/fdt_wip.c index f64139e..c2d7566 100644 --- a/libfdt/fdt_wip.c +++ b/libfdt/fdt_wip.c @@ -23,7 +23,7 @@ int fdt_setprop_inplace_namelen_partial(void *fdt, int nodeoffset, if (!propval) return proplen; - if (proplen < (len + idx)) + if ((unsigned)proplen < (len + idx)) return -FDT_ERR_NOSPACE; memcpy((char *)propval + idx, val, len); From 6c2be7d85315008be974984c0c86a8c8e55adeea Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Thu, 1 Oct 2020 17:46:29 +0100 Subject: [PATCH 018/104] libfdt: fdt_get_string(): Fix sequential write comparison warnings With -Wsign-compare, compilers warn about a mismatching signedness in comparisons in fdt_get_string(). Introduce a new usigned variable, which holds the actual (negated) stroffset value, so we avoid negating all the other variables and have proper types everywhere. Signed-off-by: Andre Przywara Message-Id: <20201001164630.4980-6-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/fdt_ro.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c index e192184..91cc6fe 100644 --- a/libfdt/fdt_ro.c +++ b/libfdt/fdt_ro.c @@ -67,11 +67,13 @@ const char *fdt_get_string(const void *fdt, int stroffset, int *lenp) len = fdt_size_dt_strings(fdt) - stroffset; } } else if (fdt_magic(fdt) == FDT_SW_MAGIC) { - if ((stroffset >= 0) - || (stroffset < -fdt_size_dt_strings(fdt))) + unsigned int sw_stroffset = -stroffset; + + if ((stroffset >= 0) || + (sw_stroffset > fdt_size_dt_strings(fdt))) goto fail; - if ((-stroffset) < len) - len = -stroffset; + if (sw_stroffset < len) + len = sw_stroffset; } else { err = -FDT_ERR_INTERNAL; goto fail; From 73e0f143b73d80889124e209548ee46834c6b559 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Thu, 1 Oct 2020 17:46:30 +0100 Subject: [PATCH 019/104] libfdt: fdt_strerror(): Fix comparison warning With -Wsign-compare, compilers warn about a mismatching signedness in a comparison in fdt_strerror(). Force FDT_ERRTABSIZE to be signed (it's surely small enough to fit), so that the types match. Also move the minus sign to errval, as this is actually what we use in the next line. Signed-off-by: Andre Przywara Message-Id: <20201001164630.4980-7-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/fdt_strerror.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libfdt/fdt_strerror.c b/libfdt/fdt_strerror.c index 768db66..b435693 100644 --- a/libfdt/fdt_strerror.c +++ b/libfdt/fdt_strerror.c @@ -40,7 +40,7 @@ static struct fdt_errtabent fdt_errtable[] = { FDT_ERRTABENT(FDT_ERR_NOPHANDLES), FDT_ERRTABENT(FDT_ERR_BADFLAGS), }; -#define FDT_ERRTABSIZE (sizeof(fdt_errtable) / sizeof(fdt_errtable[0])) +#define FDT_ERRTABSIZE ((int)(sizeof(fdt_errtable) / sizeof(fdt_errtable[0]))) const char *fdt_strerror(int errval) { @@ -48,7 +48,7 @@ const char *fdt_strerror(int errval) return ""; else if (errval == 0) return ""; - else if (errval > -FDT_ERRTABSIZE) { + else if (-errval < FDT_ERRTABSIZE) { const char *s = fdt_errtable[-errval].str; if (s) From cbca977ea121d7483b0c2351b17dca08a21cb1ca Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Mon, 28 Sep 2020 15:19:41 -0500 Subject: [PATCH 020/104] checks: Allow PCI bridge child nodes without an address Some PCI bridge nodes have child nodes such as an interrupt controller which are not PCI devices. Allow these nodes which don't have a unit-address. Signed-off-by: Rob Herring Message-Id: <20200928201942.3242124-1-robh@kernel.org> Signed-off-by: David Gibson --- checks.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/checks.c b/checks.c index b7955db..17cb689 100644 --- a/checks.c +++ b/checks.c @@ -891,10 +891,8 @@ static void check_pci_device_reg(struct check *c, struct dt_info *dti, struct no return; prop = get_property(node, "reg"); - if (!prop) { - FAIL(c, dti, node, "missing PCI reg property"); + if (!prop) return; - } cells = (cell_t *)prop->val.val; if (cells[1] || cells[2]) From b30013edb878a0e4fbe7235f38d8fd6e644479e5 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 12 Oct 2020 17:53:31 +0100 Subject: [PATCH 021/104] libfdt: Fix kernel-doc comments The API documentation in libfdt.h seems to follow the Linux kernel's kernel-doc format[1]. Running "scripts/kernel-doc -v -none" on the file reports some problems, mostly missing return values and missing parameter descriptions. Fix those up by providing the missing bits, and fixing the other small issues reported by the script. Signed-off-by: Andre Przywara [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/doc-guide/kernel-doc.rst Message-Id: <20201012165331.25016-1-andre.przywara@arm.com> Signed-off-by: David Gibson --- libfdt/libfdt.h | 111 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 75 insertions(+), 36 deletions(-) diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h index 544d3ef..5979832 100644 --- a/libfdt/libfdt.h +++ b/libfdt/libfdt.h @@ -184,23 +184,23 @@ int fdt_next_node(const void *fdt, int offset, int *depth); /** * fdt_first_subnode() - get offset of first direct subnode - * * @fdt: FDT blob * @offset: Offset of node to check - * @return offset of first subnode, or -FDT_ERR_NOTFOUND if there is none + * + * Return: offset of first subnode, or -FDT_ERR_NOTFOUND if there is none */ int fdt_first_subnode(const void *fdt, int offset); /** * fdt_next_subnode() - get offset of next direct subnode + * @fdt: FDT blob + * @offset: Offset of previous subnode * * After first calling fdt_first_subnode(), call this function repeatedly to * get direct subnodes of a parent node. * - * @fdt: FDT blob - * @offset: Offset of previous subnode - * @return offset of next subnode, or -FDT_ERR_NOTFOUND if there are no more - * subnodes + * Return: offset of next subnode, or -FDT_ERR_NOTFOUND if there are no more + * subnodes */ int fdt_next_subnode(const void *fdt, int offset); @@ -225,7 +225,6 @@ int fdt_next_subnode(const void *fdt, int offset); * Note that this is implemented as a macro and @node is used as * iterator in the loop. The parent variable be constant or even a * literal. - * */ #define fdt_for_each_subnode(node, fdt, parent) \ for (node = fdt_first_subnode(fdt, parent); \ @@ -269,17 +268,21 @@ fdt_set_hdr_(size_dt_struct); /** * fdt_header_size - return the size of the tree's header * @fdt: pointer to a flattened device tree + * + * Return: size of DTB header in bytes */ size_t fdt_header_size(const void *fdt); /** - * fdt_header_size_ - internal function which takes a version number + * fdt_header_size_ - internal function to get header size from a version number + * @version: devicetree version number + * + * Return: size of DTB header in bytes */ size_t fdt_header_size_(uint32_t version); /** * fdt_check_header - sanity check a device tree header - * @fdt: pointer to data which might be a flattened device tree * * fdt_check_header() checks that the given buffer contains what @@ -404,8 +407,7 @@ static inline uint32_t fdt_get_max_phandle(const void *fdt) * highest phandle value in the device tree blob) will be returned in the * @phandle parameter. * - * Returns: - * 0 on success or a negative error-code on failure + * Return: 0 on success or a negative error-code on failure */ int fdt_generate_phandle(const void *fdt, uint32_t *phandle); @@ -425,9 +427,11 @@ int fdt_num_mem_rsv(const void *fdt); /** * fdt_get_mem_rsv - retrieve one memory reserve map entry * @fdt: pointer to the device tree blob - * @address, @size: pointers to 64-bit variables + * @n: index of reserve map entry + * @address: pointer to 64-bit variable to hold the start address + * @size: pointer to 64-bit variable to hold the size of the entry * - * On success, *address and *size will contain the address and size of + * On success, @address and @size will contain the address and size of * the n-th reserve map entry from the device tree blob, in * native-endian format. * @@ -450,6 +454,8 @@ int fdt_get_mem_rsv(const void *fdt, int n, uint64_t *address, uint64_t *size); * namelen characters of name for matching the subnode name. This is * useful for finding subnodes based on a portion of a larger string, * such as a full path. + * + * Return: offset of the subnode or -FDT_ERR_NOTFOUND if name not found. */ #ifndef SWIG /* Not available in Python */ int fdt_subnode_offset_namelen(const void *fdt, int parentoffset, @@ -489,6 +495,8 @@ int fdt_subnode_offset(const void *fdt, int parentoffset, const char *name); * * Identical to fdt_path_offset(), but only consider the first namelen * characters of path as the path name. + * + * Return: offset of the node or negative libfdt error value otherwise */ #ifndef SWIG /* Not available in Python */ int fdt_path_offset_namelen(const void *fdt, const char *path, int namelen); @@ -588,9 +596,9 @@ int fdt_next_property_offset(const void *fdt, int offset); /** * fdt_for_each_property_offset - iterate over all properties of a node * - * @property_offset: property offset (int, lvalue) - * @fdt: FDT blob (const void *) - * @node: node offset (int) + * @property: property offset (int, lvalue) + * @fdt: FDT blob (const void *) + * @node: node offset (int) * * This is actually a wrapper around a for loop and would be used like so: * @@ -653,6 +661,9 @@ const struct fdt_property *fdt_get_property_by_offset(const void *fdt, * * Identical to fdt_get_property(), but only examine the first namelen * characters of name for matching the property name. + * + * Return: pointer to the structure representing the property, or NULL + * if not found */ #ifndef SWIG /* Not available in Python */ const struct fdt_property *fdt_get_property_namelen(const void *fdt, @@ -745,6 +756,8 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset, * * Identical to fdt_getprop(), but only examine the first namelen * characters of name for matching the property name. + * + * Return: pointer to the property's value or NULL on error */ #ifndef SWIG /* Not available in Python */ const void *fdt_getprop_namelen(const void *fdt, int nodeoffset, @@ -766,10 +779,10 @@ static inline void *fdt_getprop_namelen_w(void *fdt, int nodeoffset, * @lenp: pointer to an integer variable (will be overwritten) or NULL * * fdt_getprop() retrieves a pointer to the value of the property - * named 'name' of the node at offset nodeoffset (this will be a + * named @name of the node at offset @nodeoffset (this will be a * pointer to within the device blob itself, not a copy of the value). - * If lenp is non-NULL, the length of the property value is also - * returned, in the integer pointed to by lenp. + * If @lenp is non-NULL, the length of the property value is also + * returned, in the integer pointed to by @lenp. * * returns: * pointer to the property's value @@ -814,8 +827,11 @@ uint32_t fdt_get_phandle(const void *fdt, int nodeoffset); * @name: name of the alias th look up * @namelen: number of characters of name to consider * - * Identical to fdt_get_alias(), but only examine the first namelen - * characters of name for matching the alias name. + * Identical to fdt_get_alias(), but only examine the first @namelen + * characters of @name for matching the alias name. + * + * Return: a pointer to the expansion of the alias named @name, if it exists, + * NULL otherwise */ #ifndef SWIG /* Not available in Python */ const char *fdt_get_alias_namelen(const void *fdt, @@ -828,7 +844,7 @@ const char *fdt_get_alias_namelen(const void *fdt, * @name: name of the alias th look up * * fdt_get_alias() retrieves the value of a given alias. That is, the - * value of the property named 'name' in the node /aliases. + * value of the property named @name in the node /aliases. * * returns: * a pointer to the expansion of the alias named 'name', if it exists @@ -1004,14 +1020,13 @@ int fdt_node_offset_by_prop_value(const void *fdt, int startoffset, int fdt_node_offset_by_phandle(const void *fdt, uint32_t phandle); /** - * fdt_node_check_compatible: check a node's compatible property + * fdt_node_check_compatible - check a node's compatible property * @fdt: pointer to the device tree blob * @nodeoffset: offset of a tree node * @compatible: string to match against * - * * fdt_node_check_compatible() returns 0 if the given node contains a - * 'compatible' property with the given string as one of its elements, + * @compatible property with the given string as one of its elements, * it returns non-zero otherwise, or on error. * * returns: @@ -1075,7 +1090,7 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset, * one or more strings, each terminated by \0, as is found in a device tree * "compatible" property. * - * @return: 1 if the string is found in the list, 0 not found, or invalid list + * Return: 1 if the string is found in the list, 0 not found, or invalid list */ int fdt_stringlist_contains(const char *strlist, int listlen, const char *str); @@ -1084,7 +1099,8 @@ int fdt_stringlist_contains(const char *strlist, int listlen, const char *str); * @fdt: pointer to the device tree blob * @nodeoffset: offset of a tree node * @property: name of the property containing the string list - * @return: + * + * Return: * the number of strings in the given property * -FDT_ERR_BADVALUE if the property value is not NUL-terminated * -FDT_ERR_NOTFOUND if the property does not exist @@ -1104,7 +1120,7 @@ int fdt_stringlist_count(const void *fdt, int nodeoffset, const char *property); * small-valued cell properties, such as #address-cells, when searching for * the empty string. * - * @return: + * return: * the index of the string in the list of strings * -FDT_ERR_BADVALUE if the property value is not NUL-terminated * -FDT_ERR_NOTFOUND if the property does not exist or does not contain @@ -1128,7 +1144,7 @@ int fdt_stringlist_search(const void *fdt, int nodeoffset, const char *property, * If non-NULL, the length of the string (on success) or a negative error-code * (on failure) will be stored in the integer pointer to by lenp. * - * @return: + * Return: * A pointer to the string at the given index in the string list or NULL on * failure. On success the length of the string will be stored in the memory * location pointed to by the lenp parameter, if non-NULL. On failure one of @@ -1217,6 +1233,8 @@ int fdt_size_cells(const void *fdt, int nodeoffset); * starting from the given index, and using only the first characters * of the name. It is useful when you want to manipulate only one value of * an array and you have a string that doesn't end with \0. + * + * Return: 0 on success, negative libfdt error value otherwise */ #ifndef SWIG /* Not available in Python */ int fdt_setprop_inplace_namelen_partial(void *fdt, int nodeoffset, @@ -1330,8 +1348,13 @@ static inline int fdt_setprop_inplace_u64(void *fdt, int nodeoffset, /** * fdt_setprop_inplace_cell - change the value of a single-cell property + * @fdt: pointer to the device tree blob + * @nodeoffset: offset of the node containing the property + * @name: name of the property to change the value of + * @val: new value of the 32-bit cell * * This is an alternative name for fdt_setprop_inplace_u32() + * Return: 0 on success, negative libfdt error number otherwise. */ static inline int fdt_setprop_inplace_cell(void *fdt, int nodeoffset, const char *name, uint32_t val) @@ -1403,7 +1426,7 @@ int fdt_nop_node(void *fdt, int nodeoffset); /** * fdt_create_with_flags - begin creation of a new fdt - * @fdt: pointer to memory allocated where fdt will be created + * @buf: pointer to memory allocated where fdt will be created * @bufsize: size of the memory space at fdt * @flags: a valid combination of FDT_CREATE_FLAG_ flags, or 0. * @@ -1421,7 +1444,7 @@ int fdt_create_with_flags(void *buf, int bufsize, uint32_t flags); /** * fdt_create - begin creation of a new fdt - * @fdt: pointer to memory allocated where fdt will be created + * @buf: pointer to memory allocated where fdt will be created * @bufsize: size of the memory space at fdt * * fdt_create() is equivalent to fdt_create_with_flags() with flags=0. @@ -1486,7 +1509,8 @@ int fdt_pack(void *fdt); /** * fdt_add_mem_rsv - add one memory reserve map entry * @fdt: pointer to the device tree blob - * @address, @size: 64-bit values (native endian) + * @address: 64-bit start address of the reserve map entry + * @size: 64-bit size of the reserved region * * Adds a reserve map entry to the given blob reserving a region at * address address of length size. @@ -1691,8 +1715,14 @@ static inline int fdt_setprop_u64(void *fdt, int nodeoffset, const char *name, /** * fdt_setprop_cell - set a property to a single cell value + * @fdt: pointer to the device tree blob + * @nodeoffset: offset of the node whose property to change + * @name: name of the property to change + * @val: 32-bit integer value for the property (native endian) * * This is an alternative name for fdt_setprop_u32() + * + * Return: 0 on success, negative libfdt error value otherwise. */ static inline int fdt_setprop_cell(void *fdt, int nodeoffset, const char *name, uint32_t val) @@ -1863,8 +1893,14 @@ static inline int fdt_appendprop_u64(void *fdt, int nodeoffset, /** * fdt_appendprop_cell - append a single cell value to a property + * @fdt: pointer to the device tree blob + * @nodeoffset: offset of the node whose property to change + * @name: name of the property to change + * @val: 32-bit integer value to append to the property (native endian) * * This is an alternative name for fdt_appendprop_u32() + * + * Return: 0 on success, negative libfdt error value otherwise. */ static inline int fdt_appendprop_cell(void *fdt, int nodeoffset, const char *name, uint32_t val) @@ -1967,13 +2003,16 @@ int fdt_delprop(void *fdt, int nodeoffset, const char *name); * fdt_add_subnode_namelen - creates a new node based on substring * @fdt: pointer to the device tree blob * @parentoffset: structure block offset of a node - * @name: name of the subnode to locate + * @name: name of the subnode to create * @namelen: number of characters of name to consider * - * Identical to fdt_add_subnode(), but use only the first namelen - * characters of name as the name of the new node. This is useful for + * Identical to fdt_add_subnode(), but use only the first @namelen + * characters of @name as the name of the new node. This is useful for * creating subnodes based on a portion of a larger string, such as a * full path. + * + * Return: structure block offset of the created subnode (>=0), + * negative libfdt error value otherwise */ #ifndef SWIG /* Not available in Python */ int fdt_add_subnode_namelen(void *fdt, int parentoffset, @@ -1992,7 +2031,7 @@ int fdt_add_subnode_namelen(void *fdt, int parentoffset, * * This function will insert data into the blob, and will therefore * change the offsets of some existing nodes. - + * * returns: * structure block offset of the created nodeequested subnode (>=0), on * success From 04cf1fdc0fcf471c2e77376101bda65f727b3812 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 12 Oct 2020 17:19:39 +0100 Subject: [PATCH 022/104] convert-dtsv0: Fix signedness comparisons warning With -Wsign-compare, compilers warn about a mismatching signedness in comparisons in the generated lexer code. In this case we walk over an array, and never use negative indicies, so we can change the loop counter variable to be unsigned. This fixes "make convert-dtsv0", when compiled with -Wsign-compare. Signed-off-by: Andre Przywara Message-Id: <20201012161948.23994-3-andre.przywara@arm.com> Signed-off-by: David Gibson --- convert-dtsv0-lexer.l | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/convert-dtsv0-lexer.l b/convert-dtsv0-lexer.l index f52e8a1..7a66f57 100644 --- a/convert-dtsv0-lexer.l +++ b/convert-dtsv0-lexer.l @@ -94,7 +94,7 @@ static const struct { [0-9a-fA-F]+ { unsigned long long val; int obase = 16, width = 0; - int i; + unsigned int i; val = strtoull(yytext, NULL, cbase); From e1147b159e9209e1c3102f350445ba9927048b4d Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 12 Oct 2020 17:19:43 +0100 Subject: [PATCH 023/104] dtc: Fix signedness comparisons warnings: change types With -Wsign-compare, compilers warn about a mismatching signedness in comparisons in various parts of dtc. Many variables are using signed types unnecessarily, as we never use negative value in them. Change their types to be unsigned, to prevent issues with comparisons. Signed-off-by: Andre Przywara Message-Id: <20201012161948.23994-7-andre.przywara@arm.com> Signed-off-by: David Gibson --- data.c | 4 ++-- dtc.h | 8 ++++---- flattree.c | 8 ++++---- livetree.c | 2 +- yamltree.c | 6 +++--- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/data.c b/data.c index 0a43b6d..fdb0407 100644 --- a/data.c +++ b/data.c @@ -21,10 +21,10 @@ void data_free(struct data d) free(d.val); } -struct data data_grow_for(struct data d, int xlen) +struct data data_grow_for(struct data d, unsigned int xlen) { struct data nd; - int newsize; + unsigned int newsize; if (xlen == 0) return d; diff --git a/dtc.h b/dtc.h index a08f415..d3e82fb 100644 --- a/dtc.h +++ b/dtc.h @@ -105,13 +105,13 @@ extern const char *markername(enum markertype markertype); struct marker { enum markertype type; - int offset; + unsigned int offset; char *ref; struct marker *next; }; struct data { - int len; + unsigned int len; char *val; struct marker *markers; }; @@ -129,7 +129,7 @@ size_t type_marker_length(struct marker *m); void data_free(struct data d); -struct data data_grow_for(struct data d, int xlen); +struct data data_grow_for(struct data d, unsigned int xlen); struct data data_copy_mem(const char *mem, int len); struct data data_copy_escape_string(const char *s, int len); @@ -253,7 +253,7 @@ void append_to_property(struct node *node, const char *get_unitname(struct node *node); struct property *get_property(struct node *node, const char *propname); cell_t propval_cell(struct property *prop); -cell_t propval_cell_n(struct property *prop, int n); +cell_t propval_cell_n(struct property *prop, unsigned int n); struct property *get_property_by_label(struct node *tree, const char *label, struct node **node); struct marker *get_marker_label(struct node *tree, const char *label, diff --git a/flattree.c b/flattree.c index 07f10d2..4659afb 100644 --- a/flattree.c +++ b/flattree.c @@ -149,7 +149,7 @@ static void asm_emit_align(void *e, int a) static void asm_emit_data(void *e, struct data d) { FILE *f = e; - int off = 0; + unsigned int off = 0; struct marker *m = d.markers; for_each_marker_of_type(m, LABEL) @@ -219,7 +219,7 @@ static struct emitter asm_emitter = { static int stringtable_insert(struct data *d, const char *str) { - int i; + unsigned int i; /* FIXME: do this more efficiently? */ @@ -345,7 +345,7 @@ static void make_fdt_header(struct fdt_header *fdt, void dt_to_blob(FILE *f, struct dt_info *dti, int version) { struct version_info *vi = NULL; - int i; + unsigned int i; struct data blob = empty_data; struct data reservebuf = empty_data; struct data dtbuf = empty_data; @@ -446,7 +446,7 @@ static void dump_stringtable_asm(FILE *f, struct data strbuf) void dt_to_asm(FILE *f, struct dt_info *dti, int version) { struct version_info *vi = NULL; - int i; + unsigned int i; struct data strbuf = empty_data; struct reserve_info *re; const char *symprefix = "dt"; diff --git a/livetree.c b/livetree.c index 032df58..7eacd02 100644 --- a/livetree.c +++ b/livetree.c @@ -438,7 +438,7 @@ cell_t propval_cell(struct property *prop) return fdt32_to_cpu(*((fdt32_t *)prop->val.val)); } -cell_t propval_cell_n(struct property *prop, int n) +cell_t propval_cell_n(struct property *prop, unsigned int n) { assert(prop->val.len / sizeof(cell_t) >= n); return fdt32_to_cpu(*((fdt32_t *)prop->val.val + n)); diff --git a/yamltree.c b/yamltree.c index 4e93c12..e63d32f 100644 --- a/yamltree.c +++ b/yamltree.c @@ -29,11 +29,11 @@ char *yaml_error_name[] = { (emitter)->problem, __func__, __LINE__); \ }) -static void yaml_propval_int(yaml_emitter_t *emitter, struct marker *markers, char *data, int len, int width) +static void yaml_propval_int(yaml_emitter_t *emitter, struct marker *markers, char *data, unsigned int len, int width) { yaml_event_t event; void *tag; - int off, start_offset = markers->offset; + unsigned int off, start_offset = markers->offset; switch(width) { case 1: tag = "!u8"; break; @@ -112,7 +112,7 @@ static void yaml_propval_string(yaml_emitter_t *emitter, char *str, int len) static void yaml_propval(yaml_emitter_t *emitter, struct property *prop) { yaml_event_t event; - int len = prop->val.len; + unsigned int len = prop->val.len; struct marker *m = prop->val.markers; /* Emit the property name */ From 3bc3a6b9fe0cba171bef7d1cc2b04a362228dd1c Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 12 Oct 2020 17:19:45 +0100 Subject: [PATCH 024/104] dtc: Fix signedness comparisons warnings: Wrap (-1) With -Wsign-compare, compilers warn about a mismatching signedness in a comparison in dtc's data_copy_file(). Even though maxlen is of an unsigned type, we compare against "-1", which is passed in from the parser to indicate an unknown size. Cast the "-1" to an unsigned size to make the comparison match. Signed-off-by: Andre Przywara Message-Id: <20201012161948.23994-9-andre.przywara@arm.com> Signed-off-by: David Gibson --- data.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data.c b/data.c index fdb0407..1473423 100644 --- a/data.c +++ b/data.c @@ -84,7 +84,7 @@ struct data data_copy_file(FILE *f, size_t maxlen) while (!feof(f) && (d.len < maxlen)) { size_t chunksize, ret; - if (maxlen == -1) + if (maxlen == (size_t)-1) chunksize = 4096; else chunksize = maxlen - d.len; From 05874d08212d23dafd753ab52a5762c9b69b25de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 12 Oct 2020 11:34:03 +0400 Subject: [PATCH 025/104] pylibfdt: allow build out of tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With meson, we have to support out-of-tree build. Introduce a --top-builddir option, which will default to the current directory to lookup generated filed such as version_gen.h and output directories. Other source paths are derived from the location of the setup.py script in the source tree. --build-lib is changed to be relative to the current directory, instead of relative to setup.py. This has less surprising results! Signed-off-by: Marc-André Lureau Message-Id: <20201012073405.1682782-2-marcandre.lureau@redhat.com> Signed-off-by: David Gibson --- pylibfdt/Makefile.pylibfdt | 4 ++-- pylibfdt/setup.py | 27 +++++++++++++++++++-------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt index 6866a0b..1b5f236 100644 --- a/pylibfdt/Makefile.pylibfdt +++ b/pylibfdt/Makefile.pylibfdt @@ -10,7 +10,7 @@ PYLIBFDT_CLEANDIRS_L = build __pycache__ PYLIBFDT_CLEANDIRS = $(PYLIBFDT_CLEANDIRS_L:%=$(PYLIBFDT_dir)/%) SETUP = $(PYLIBFDT_dir)/setup.py -SETUPFLAGS = +SETUPFLAGS = --top-builddir . ifndef V SETUPFLAGS += --quiet @@ -18,7 +18,7 @@ endif $(PYMODULE): $(PYLIBFDT_srcs) $(LIBFDT_archive) $(SETUP) $(VERSION_FILE) @$(VECHO) PYMOD $@ - $(PYTHON) $(SETUP) $(SETUPFLAGS) build_ext --build-lib=../$(PYLIBFDT_dir) + $(PYTHON) $(SETUP) $(SETUPFLAGS) build_ext --build-lib=$(PYLIBFDT_dir) install_pylibfdt: $(PYMODULE) @$(VECHO) INSTALL-PYLIB diff --git a/pylibfdt/setup.py b/pylibfdt/setup.py index 53f2bef..ef40f15 100755 --- a/pylibfdt/setup.py +++ b/pylibfdt/setup.py @@ -19,23 +19,33 @@ import sys VERSION_PATTERN = '^#define DTC_VERSION "DTC ([^"]*)"$' +def get_top_builddir(): + if '--top-builddir' in sys.argv: + index = sys.argv.index('--top-builddir') + sys.argv.pop(index) + return sys.argv.pop(index) + else: + return os.getcwd() + + +srcdir = os.path.dirname(os.path.abspath(sys.argv[0])) +top_builddir = get_top_builddir() + + def get_version(): - version_file = "../version_gen.h" + version_file = os.path.join(top_builddir, 'version_gen.h') f = open(version_file, 'rt') m = re.match(VERSION_PATTERN, f.readline()) return m.group(1) -setupdir = os.path.dirname(os.path.abspath(sys.argv[0])) -os.chdir(setupdir) - libfdt_module = Extension( '_libfdt', - sources=['libfdt.i'], - include_dirs=['../libfdt'], + sources=[os.path.join(srcdir, 'libfdt.i')], + include_dirs=[os.path.join(srcdir, '../libfdt')], libraries=['fdt'], - library_dirs=['../libfdt'], - swig_opts=['-I../libfdt'], + library_dirs=[os.path.join(top_builddir, 'libfdt')], + swig_opts=['-I' + os.path.join(srcdir, '../libfdt')], ) setup( @@ -44,5 +54,6 @@ setup( author='Simon Glass ', description='Python binding for libfdt', ext_modules=[libfdt_module], + package_dir={'': srcdir}, py_modules=['libfdt'], ) From 67849a327927e1028010e973b36f7b1280ce8184 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 12 Oct 2020 11:34:04 +0400 Subject: [PATCH 026/104] build-sys: add meson build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The meson build system allows projects to "vendor" dtc easily, thanks to subproject(). QEMU has recently switched to meson, and adding meson support to dtc will help to handle the QEMU submodule. meson rules are arguably simpler to write and maintain than the hand-crafted/custom Makefile. meson support various backends, and default build options (including coverage, sanitizer, debug/release etc, see: https://mesonbuild.com/Builtin-options.html) Compare to the Makefiles, the same build targets should be built and installed and the same tests should be run ("meson test" can be provided extra test arguments for running the equivalent of checkm/checkv). There is no support EXTRAVERSION/LOCAL_VERSION/CONFIG_LOCALVERSION, instead the version is simply set with project(), and vcs_tag() is used for git/dirty version reporting (This is most common and is hopefully enough. If necessary, configure-time options could be added for extra versioning.). libfdt shared library is build following regular naming conventions: instead of libfdt.so.1 -> libfdt-1.6.0.so (with current build-sys), libfdt.so.1 -> libfdt.so.1.6.0. I am not sure why the current build system use an uncommon naming pattern. I also included a libfdt.pc pkg-config file, as convenience. Both Linux native build and mingw cross-build pass. CI pass. Tests are only run on native build. The current Makefiles are left in-tree, and make/check still work. Eventually, the Makefiles could be marked as deprecated, to start a transition period and avoid having to maintain 2 build systems in the near future. (run_tests.sh could eventually be replaced by the meson test runner, which would have several advantages in term of flexibility/features, but this is left for another day) Signed-off-by: Marc-André Lureau Message-Id: <20201012073405.1682782-3-marcandre.lureau@redhat.com> Signed-off-by: David Gibson --- libfdt/meson.build | 50 +++++++++++++++++ meson.build | 127 ++++++++++++++++++++++++++++++++++++++++++ meson_options.txt | 10 ++++ pylibfdt/meson.build | 13 +++++ tests/meson.build | 130 +++++++++++++++++++++++++++++++++++++++++++ version_gen.h.in | 1 + 6 files changed, 331 insertions(+) create mode 100644 libfdt/meson.build create mode 100644 meson.build create mode 100644 meson_options.txt create mode 100644 pylibfdt/meson.build create mode 100644 tests/meson.build create mode 100644 version_gen.h.in diff --git a/libfdt/meson.build b/libfdt/meson.build new file mode 100644 index 0000000..0307ffb --- /dev/null +++ b/libfdt/meson.build @@ -0,0 +1,50 @@ +version_script = '-Wl,--version-script=@0@'.format(meson.current_source_dir() / 'version.lds') +if not cc.has_link_argument(version_script) + version_script = [] +endif + +sources = files( + 'fdt.c', + 'fdt_addresses.c', + 'fdt_check.c', + 'fdt_empty_tree.c', + 'fdt_overlay.c', + 'fdt_ro.c', + 'fdt_rw.c', + 'fdt_strerror.c', + 'fdt_sw.c', + 'fdt_wip.c', +) + +libfdt = library( + 'fdt', sources, + version: '1.6.0', + link_args: ['-Wl,--no-undefined', version_script], + link_depends: 'version.lds', + install: true, +) + +libfdt_inc = include_directories('.') + +libfdt_dep = declare_dependency( + include_directories: libfdt_inc, + link_with: libfdt, +) + +install_headers( + files( + 'fdt.h', + 'libfdt.h', + 'libfdt_env.h', + ) +) + +pkgconfig = import('pkgconfig') + +pkgconfig.generate( + libraries: libfdt, + version: meson.project_version(), + filebase: 'libfdt', + name: 'libfdt', + description: 'Flat Device Tree manipulation', +) diff --git a/meson.build b/meson.build new file mode 100644 index 0000000..2c65104 --- /dev/null +++ b/meson.build @@ -0,0 +1,127 @@ +project('dtc', 'c', + version: '1.6.0', + license: ['GPL2+', 'BSD-2'], + default_options: 'werror=true', +) + +cc = meson.get_compiler('c') + +add_project_arguments(cc.get_supported_arguments([ + '-Wall', + '-Wpointer-arith', + '-Wcast-qual', + '-Wnested-externs', + '-Wstrict-prototypes', + '-Wmissing-prototypes', + '-Wredundant-decls', + '-Wshadow' +]),language: 'c') + +if host_machine.system() == 'windows' + add_project_arguments( + '-D__USE_MINGW_ANSI_STDIO=1', + language: 'c' + ) +endif + +add_project_arguments( + '-DFDT_ASSUME_MASK=' + get_option('assume-mask').to_string(), + language: 'c' +) + +yamltree = 'yamltree.c' +yaml = dependency('yaml-0.1', required: get_option('yaml')) +if not yaml.found() + add_project_arguments('-DNO_YAML', language: 'c') + yamltree = [] +endif + +valgrind = dependency('valgrind', required: get_option('valgrind')) +if not valgrind.found() + add_project_arguments('-DNO_VALGRIND', language: 'c') +endif + +py = import('python') +py = py.find_installation(required: get_option('python')) +swig = find_program('swig', required: get_option('python')) + +version_gen_h = vcs_tag( + input: 'version_gen.h.in', + output: 'version_gen.h', +) + +subdir('libfdt') + +if get_option('tools') + flex = find_program('flex', required: true) + bison = find_program('bison', required: true) + + util_dep = declare_dependency( + sources: ['util.c', version_gen_h], + include_directories: '.', + dependencies: libfdt_dep + ) + + lgen = generator( + flex, + output: '@PLAINNAME@.lex.c', + arguments: ['-o', '@OUTPUT@', '@INPUT@'], + ) + + pgen = generator( + bison, + output: ['@BASENAME@.tab.c', '@BASENAME@.tab.h'], + arguments: ['@INPUT@', '--defines=@OUTPUT1@', '--output=@OUTPUT0@'], + ) + + if cc.check_header('fnmatch.h') + executable( + 'convert-dtsv0', + [ + lgen.process('convert-dtsv0-lexer.l'), + 'srcpos.c', + ], + dependencies: util_dep, + install: true, + ) + endif + + executable( + 'dtc', + [ + lgen.process('dtc-lexer.l'), + pgen.process('dtc-parser.y'), + 'checks.c', + 'data.c', + 'dtc.c', + 'flattree.c', + 'fstree.c', + 'livetree.c', + 'srcpos.c', + 'treesource.c', + yamltree, + ], + dependencies: [util_dep, yaml], + install: true, + ) + + foreach e: ['fdtdump', 'fdtget', 'fdtput', 'fdtoverlay'] + executable(e, files(e + '.c'), dependencies: util_dep, install: true) + endforeach + + install_data( + 'dtdiff', + install_dir: get_option('prefix') / get_option('bindir'), + install_mode: 'rwxr-xr-x', + ) +endif + +if not meson.is_cross_build() + if py.found() and swig.found() + subdir('pylibfdt') + endif + + if get_option('tools') + subdir('tests') + endif +endif diff --git a/meson_options.txt b/meson_options.txt new file mode 100644 index 0000000..ea59c28 --- /dev/null +++ b/meson_options.txt @@ -0,0 +1,10 @@ +option('tools', type: 'boolean', value: true, + description: 'Build tools') +option('assume-mask', type: 'integer', value: 0, + description: 'Control the assumptions made (e.g. risking security issues) in the code.') +option('yaml', type: 'feature', value: 'auto', + description: 'YAML support') +option('valgrind', type: 'feature', value: 'auto', + description: 'Valgrind support') +option('python', type: 'feature', value: 'auto', + description: 'Build pylibfdt Python library') diff --git a/pylibfdt/meson.build b/pylibfdt/meson.build new file mode 100644 index 0000000..088f249 --- /dev/null +++ b/pylibfdt/meson.build @@ -0,0 +1,13 @@ +setup_py = find_program('setup.py') +setup_py = [setup_py.path(), '--quiet', '--top-builddir', meson.current_build_dir() / '..'] + +custom_target( + 'pylibfdt', + input: 'libfdt.i', + output: '_libfdt.so', + depends: version_gen_h, + command: [setup_py, 'build_ext', '--build-lib=' + meson.current_build_dir()], + build_by_default: true, +) + +meson.add_install_script(setup_py, 'install', '--prefix=' + get_option('prefix'), '--root=$DESTDIR') diff --git a/tests/meson.build b/tests/meson.build new file mode 100644 index 0000000..8976dc1 --- /dev/null +++ b/tests/meson.build @@ -0,0 +1,130 @@ +trees = static_library('trees', files('trees.S'), c_args: '-D__ASSEMBLY__', + include_directories: libfdt_inc) + +dumptrees = executable('dumptrees', files('dumptrees.c'), + link_with: trees, dependencies: libfdt_dep) + +dumptrees_dtb = custom_target( + 'dumptrees', + command: [dumptrees, meson.current_build_dir()], + output: [ + 'test_tree1.dtb', + 'bad_node_char.dtb', + 'bad_node_format.dtb', + 'bad_prop_char.dtb', + 'ovf_size_strings.dtb', + 'truncated_property.dtb', + 'truncated_string.dtb', + 'truncated_memrsv.dtb', + ] +) + +testutil_dep = declare_dependency(sources: ['testutils.c'], link_with: trees) + +tests = [ + 'add_subnode_with_nops', + 'addr_size_cells', + 'addr_size_cells2', + 'appendprop1', + 'appendprop2', + 'appendprop_addrrange', + 'boot-cpuid', + 'char_literal', + 'check_full', + 'check_header', + 'check_path', + 'del_node', + 'del_property', + 'dtb_reverse', + 'dtbs_equal_ordered', + 'dtbs_equal_unordered', + 'extra-terminating-null', + 'find_property', + 'fs_tree1', + 'get_alias', + 'get_mem_rsv', + 'get_name', + 'get_path', + 'get_phandle', + 'get_prop_offset', + 'getprop', + 'incbin', + 'integer-expressions', + 'mangle-layout', + 'move_and_save', + 'node_check_compatible', + 'node_offset_by_compatible', + 'node_offset_by_phandle', + 'node_offset_by_prop_value', + 'nop_node', + 'nop_property', + 'nopulate', + 'notfound', + 'open_pack', + 'overlay', + 'overlay_bad_fixup', + 'parent_offset', + 'path-references', + 'path_offset', + 'path_offset_aliases', + 'phandle_format', + 'property_iterate', + 'propname_escapes', + 'references', + 'root_node', + 'rw_oom', + 'rw_tree1', + 'set_name', + 'setprop', + 'setprop_inplace', + 'sized_cells', + 'string_escapes', + 'stringlist', + 'subnode_iterate', + 'subnode_offset', + 'supernode_atdepth_offset', + 'sw_states', + 'sw_tree1', + 'utilfdt_test', +] + +tests += [ + 'truncated_memrsv', + 'truncated_property', + 'truncated_string', +] + +dl = cc.find_library('dl', required: false) +if dl.found() + tests += [ + 'asm_tree_dump', + 'value-labels', + ] +endif + +foreach t: tests + executable(t, files(t + '.c'), dependencies: [testutil_dep, util_dep, libfdt_dep, dl]) +endforeach + +run_tests = find_program('run_tests.sh') + + +env = [ + 'PYTHON=' + py.path(), + 'PYTHONPATH=' + meson.source_root() / 'pylibfdt', +] + +if not py.found() + env += 'NO_PYTHON=1' +endif +if not yaml.found() + env += 'NO_YAML=1' +endif + +test( + 'run-test', + run_tests, + workdir: meson.current_build_dir(), + depends: dumptrees_dtb, + env: env, +) diff --git a/version_gen.h.in b/version_gen.h.in new file mode 100644 index 0000000..7771abb --- /dev/null +++ b/version_gen.h.in @@ -0,0 +1 @@ +#define DTC_VERSION "DTC @VCS_TAG@" From 5e735860c4786418c12751de639b519891a61192 Mon Sep 17 00:00:00 2001 From: Tom Rini Date: Wed, 4 Nov 2020 08:06:05 -0500 Subject: [PATCH 027/104] libfdt: Check for 8-byte address alignment in fdt_ro_probe_() The device tree must be loaded in to memory at an 8-byte aligned address. Add a check for this condition in fdt_ro_probe_() and a new error code to return if we are not. Signed-off-by: Tom Rini Message-Id: <20201104130605.28874-1-trini@konsulko.com> Signed-off-by: David Gibson --- libfdt/fdt.c | 4 ++++ libfdt/libfdt.h | 6 +++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/libfdt/fdt.c b/libfdt/fdt.c index 6cf2fa0..3e89307 100644 --- a/libfdt/fdt.c +++ b/libfdt/fdt.c @@ -22,6 +22,10 @@ int32_t fdt_ro_probe_(const void *fdt) if (can_assume(VALID_DTB)) return totalsize; + /* The device tree must be at an 8-byte aligned address */ + if ((uintptr_t)fdt & 7) + return -FDT_ERR_ALIGNMENT; + if (fdt_magic(fdt) == FDT_MAGIC) { /* Complete tree */ if (!can_assume(LATEST)) { diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h index 5979832..89adee3 100644 --- a/libfdt/libfdt.h +++ b/libfdt/libfdt.h @@ -101,7 +101,11 @@ extern "C" { /* FDT_ERR_BADFLAGS: The function was passed a flags field that * contains invalid flags or an invalid combination of flags. */ -#define FDT_ERR_MAX 18 +#define FDT_ERR_ALIGNMENT 19 + /* FDT_ERR_ALIGNMENT: The device tree base address is not 8-byte + * aligned. */ + +#define FDT_ERR_MAX 19 /* constants */ #define FDT_MAX_PHANDLE 0xfffffffe From 30a56bce4f0bdf59ca52af244d4da5250924bfdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 7 Dec 2020 17:00:51 +0400 Subject: [PATCH 028/104] meson: fix -Wall warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Meson already handles Wall via the built-in warning_level option. Signed-off-by: Marc-André Lureau Message-Id: <20201207130055.462734-2-marcandre.lureau@redhat.com> Signed-off-by: David Gibson --- meson.build | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/meson.build b/meson.build index 2c65104..b23ea1b 100644 --- a/meson.build +++ b/meson.build @@ -6,16 +6,18 @@ project('dtc', 'c', cc = meson.get_compiler('c') -add_project_arguments(cc.get_supported_arguments([ - '-Wall', - '-Wpointer-arith', - '-Wcast-qual', - '-Wnested-externs', - '-Wstrict-prototypes', - '-Wmissing-prototypes', - '-Wredundant-decls', - '-Wshadow' -]),language: 'c') +add_project_arguments( + cc.get_supported_arguments([ + '-Wpointer-arith', + '-Wcast-qual', + '-Wnested-externs', + '-Wstrict-prototypes', + '-Wmissing-prototypes', + '-Wredundant-decls', + '-Wshadow' + ]), + language: 'c' +) if host_machine.system() == 'windows' add_project_arguments( From f8b46098824d846675103d9b1ce0a3f7b4548623 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 7 Dec 2020 17:00:52 +0400 Subject: [PATCH 029/104] meson: do not assume python is installed, skip tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marc-André Lureau Message-Id: <20201207130055.462734-3-marcandre.lureau@redhat.com> Signed-off-by: David Gibson --- tests/meson.build | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/meson.build b/tests/meson.build index 8976dc1..9d3a4e7 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -108,14 +108,14 @@ endforeach run_tests = find_program('run_tests.sh') - -env = [ - 'PYTHON=' + py.path(), - 'PYTHONPATH=' + meson.source_root() / 'pylibfdt', -] - +env = [] if not py.found() env += 'NO_PYTHON=1' +else + env += [ + 'PYTHON=' + py.path(), + 'PYTHONPATH=' + meson.source_root() / 'pylibfdt', + ] endif if not yaml.found() env += 'NO_YAML=1' From bab85e48a6f4c32a9e8ab201cdaf4a0568c74da7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 7 Dec 2020 17:00:53 +0400 Subject: [PATCH 030/104] meson: increase default timeout for tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marc-André Lureau Message-Id: <20201207130055.462734-4-marcandre.lureau@redhat.com> Signed-off-by: David Gibson --- tests/meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/meson.build b/tests/meson.build index 9d3a4e7..fa06824 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -127,4 +127,5 @@ test( workdir: meson.current_build_dir(), depends: dumptrees_dtb, env: env, + timeout: 1800, # mostly for valgrind ) From a7c40409934971ac1bd934ccc411bc6932b86564 Mon Sep 17 00:00:00 2001 From: Tom Rini Date: Thu, 10 Dec 2020 21:27:36 -0500 Subject: [PATCH 031/104] libfdt: Internally perform potentially unaligned loads Commits 6dcb8ba4 "libfdt: Add helpers for accessing unaligned words" introduced changes to support unaligned reads for ARM platforms and 11738cf01f15 "libfdt: Don't use memcpy to handle unaligned reads on ARM" improved the performance of these helpers. On further discussion, while there are potential cases where we could be used on platforms that do not fixup unaligned reads for us, making this choice the default is very expensive in terms of binary size and access time. To address this, introduce and use new fdt{32,64}_ld_ functions that call fdt{32,64}_to_cpu() as was done prior to the above mentioned commits. Leave the existing load functions as unaligned-safe and include comments in both cases. Reviewed-by: Rob Herring Signed-off-by: Tom Rini Message-Id: <20201211022736.31657-1-trini@konsulko.com> Tested-by: John Paul Adrian Glaubitz Signed-off-by: David Gibson --- libfdt/fdt_ro.c | 20 ++++++++++---------- libfdt/libfdt.h | 8 +++----- libfdt/libfdt_internal.h | 19 +++++++++++++++++++ 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c index 91cc6fe..17584da 100644 --- a/libfdt/fdt_ro.c +++ b/libfdt/fdt_ro.c @@ -181,8 +181,8 @@ int fdt_get_mem_rsv(const void *fdt, int n, uint64_t *address, uint64_t *size) if (!can_assume(VALID_INPUT) && !re) return -FDT_ERR_BADOFFSET; - *address = fdt64_ld(&re->address); - *size = fdt64_ld(&re->size); + *address = fdt64_ld_(&re->address); + *size = fdt64_ld_(&re->size); return 0; } @@ -192,7 +192,7 @@ int fdt_num_mem_rsv(const void *fdt) const struct fdt_reserve_entry *re; for (i = 0; (re = fdt_mem_rsv(fdt, i)) != NULL; i++) { - if (fdt64_ld(&re->size) == 0) + if (fdt64_ld_(&re->size) == 0) return i; } return -FDT_ERR_TRUNCATED; @@ -370,7 +370,7 @@ static const struct fdt_property *fdt_get_property_by_offset_(const void *fdt, prop = fdt_offset_ptr_(fdt, offset); if (lenp) - *lenp = fdt32_ld(&prop->len); + *lenp = fdt32_ld_(&prop->len); return prop; } @@ -408,7 +408,7 @@ static const struct fdt_property *fdt_get_property_namelen_(const void *fdt, offset = -FDT_ERR_INTERNAL; break; } - if (fdt_string_eq_(fdt, fdt32_ld(&prop->nameoff), + if (fdt_string_eq_(fdt, fdt32_ld_(&prop->nameoff), name, namelen)) { if (poffset) *poffset = offset; @@ -461,7 +461,7 @@ const void *fdt_getprop_namelen(const void *fdt, int nodeoffset, /* Handle realignment */ if (!can_assume(LATEST) && fdt_version(fdt) < 0x10 && - (poffset + sizeof(*prop)) % 8 && fdt32_ld(&prop->len) >= 8) + (poffset + sizeof(*prop)) % 8 && fdt32_ld_(&prop->len) >= 8) return prop->data + 4; return prop->data; } @@ -479,7 +479,7 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset, int namelen; if (!can_assume(VALID_INPUT)) { - name = fdt_get_string(fdt, fdt32_ld(&prop->nameoff), + name = fdt_get_string(fdt, fdt32_ld_(&prop->nameoff), &namelen); if (!name) { if (lenp) @@ -488,13 +488,13 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset, } *namep = name; } else { - *namep = fdt_string(fdt, fdt32_ld(&prop->nameoff)); + *namep = fdt_string(fdt, fdt32_ld_(&prop->nameoff)); } } /* Handle realignment */ if (!can_assume(LATEST) && fdt_version(fdt) < 0x10 && - (offset + sizeof(*prop)) % 8 && fdt32_ld(&prop->len) >= 8) + (offset + sizeof(*prop)) % 8 && fdt32_ld_(&prop->len) >= 8) return prop->data + 4; return prop->data; } @@ -519,7 +519,7 @@ uint32_t fdt_get_phandle(const void *fdt, int nodeoffset) return 0; } - return fdt32_ld(php); + return fdt32_ld_(php); } const char *fdt_get_alias_namelen(const void *fdt, diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h index 89adee3..2bc16a8 100644 --- a/libfdt/libfdt.h +++ b/libfdt/libfdt.h @@ -126,12 +126,10 @@ static inline void *fdt_offset_ptr_w(void *fdt, int offset, int checklen) uint32_t fdt_next_tag(const void *fdt, int offset, int *nextoffset); /* - * Alignment helpers: - * These helpers access words from a device tree blob. They're - * built to work even with unaligned pointers on platforms (ike - * ARM) that don't like unaligned loads and stores + * External helpers to access words from a device tree blob. They're built + * to work even with unaligned pointers on platforms (such as ARMv5) that don't + * like unaligned loads and stores. */ - static inline uint32_t fdt32_ld(const fdt32_t *p) { const uint8_t *bp = (const uint8_t *)p; diff --git a/libfdt/libfdt_internal.h b/libfdt/libfdt_internal.h index d4e0bd4..8b580aa 100644 --- a/libfdt/libfdt_internal.h +++ b/libfdt/libfdt_internal.h @@ -46,6 +46,25 @@ static inline struct fdt_reserve_entry *fdt_mem_rsv_w_(void *fdt, int n) return (void *)(uintptr_t)fdt_mem_rsv_(fdt, n); } +/* + * Internal helpers to access tructural elements of the device tree blob + * (rather than for exaple reading integers from within property values). We + * assume that we are either given a naturally aligned address for the platform + * or if we are not, we are on a platform where unaligned memory reads will be + * handled in a graceful manner. If this is not the case there are _unaligned + * versions of these functions that follow and can be used. + * + */ +static inline uint32_t fdt32_ld_(const fdt32_t *p) +{ + return fdt32_to_cpu(*p); +} + +static inline uint64_t fdt64_ld_(const fdt64_t *p) +{ + return fdt64_to_cpu(*p); +} + #define FDT_SW_MAGIC (~FDT_MAGIC) /**********************************************************************/ From 7cd5d5fe43d56e3017397fa3a086096e11920c3e Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 16 Dec 2020 16:51:38 +1100 Subject: [PATCH 032/104] libfdt: Tweak description of assume-aligned load helpers There's a small inaccuracy in the comment describing these new helpers. This corrects it, and reformats while we're there. Fixes: f98f28ab ("libfdt: Internally perform potentially unaligned loads") Signed-off-by: David Gibson --- libfdt/libfdt_internal.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libfdt/libfdt_internal.h b/libfdt/libfdt_internal.h index 8b580aa..16bda19 100644 --- a/libfdt/libfdt_internal.h +++ b/libfdt/libfdt_internal.h @@ -47,13 +47,13 @@ static inline struct fdt_reserve_entry *fdt_mem_rsv_w_(void *fdt, int n) } /* - * Internal helpers to access tructural elements of the device tree blob - * (rather than for exaple reading integers from within property values). We - * assume that we are either given a naturally aligned address for the platform - * or if we are not, we are on a platform where unaligned memory reads will be - * handled in a graceful manner. If this is not the case there are _unaligned - * versions of these functions that follow and can be used. - * + * Internal helpers to access tructural elements of the device tree + * blob (rather than for exaple reading integers from within property + * values). We assume that we are either given a naturally aligned + * address for the platform or if we are not, we are on a platform + * where unaligned memory reads will be handled in a graceful manner. + * If not the external helpers fdtXX_ld() from libfdt.h can be used + * instead. */ static inline uint32_t fdt32_ld_(const fdt32_t *p) { From f7e5737f26aa7198db476c94463766910db48941 Mon Sep 17 00:00:00 2001 From: Paul Barker Date: Sat, 19 Dec 2020 14:35:21 +0000 Subject: [PATCH 033/104] tests: Fix overlay_overlay_nosugar test case This test was accidentally skipped as the wrong test dts file was built. The fragment numbering in this sugar-free test case needed adjusting to match the numbering generated by dtc for overlay_overlay.dts. Signed-off-by: Paul Barker Message-Id: <20201219143521.2118-1-pbarker@konsulko.com> Signed-off-by: David Gibson --- tests/overlay_overlay_nosugar.dts | 8 ++++---- tests/run_tests.sh | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/overlay_overlay_nosugar.dts b/tests/overlay_overlay_nosugar.dts index b6d841b..b5947e9 100644 --- a/tests/overlay_overlay_nosugar.dts +++ b/tests/overlay_overlay_nosugar.dts @@ -48,7 +48,7 @@ }; }; - fragment@5 { + fragment@4 { target = <&test>; __overlay__ { @@ -58,7 +58,7 @@ }; }; - fragment@6 { + fragment@5 { target = <&test>; __overlay__ { @@ -66,7 +66,7 @@ }; }; - fragment@7 { + fragment@6 { target = <&test>; __overlay__ { @@ -74,7 +74,7 @@ }; }; - fragment@8 { + fragment@7 { target = <&test>; __overlay__ { diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 294585b..4b8dada 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -288,7 +288,7 @@ dtc_overlay_tests () { run_test check_path overlay_overlay.test.dtb exists "/__local_fixups__" # Without syntactic sugar - run_dtc_test -I dts -O dtb -o overlay_overlay_nosugar.test.dtb "$SRCDIR/overlay_overlay.dts" + run_dtc_test -I dts -O dtb -o overlay_overlay_nosugar.test.dtb "$SRCDIR/overlay_overlay_nosugar.dts" run_test check_path overlay_overlay_nosugar.test.dtb not-exists "/__symbols__" run_test check_path overlay_overlay_nosugar.test.dtb exists "/__fixups__" run_test check_path overlay_overlay_nosugar.test.dtb exists "/__local_fixups__" From 3b01518e688d7c7d9d46ac41b11dd2b0c4788775 Mon Sep 17 00:00:00 2001 From: Justin Covell Date: Mon, 28 Dec 2020 20:17:49 -0800 Subject: [PATCH 034/104] Set last_comp_version correctly in new dtb and fix potential version issues in fdt_open_into Changes in v3: - Remove noop version sets - Set version correctly on loaded fdt in fdt_open_into Fixes: f1879e1a50eb ("Add limited read-only support for older (V2 and V3) device tree to libfdt.") Signed-off-by: Justin Covell Message-Id: <20201229041749.2187-1-jujugoboom@gmail.com> Signed-off-by: David Gibson --- libfdt/fdt_rw.c | 4 +++- libfdt/fdt_sw.c | 2 +- libfdt/libfdt.h | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c index 68887b9..f13458d 100644 --- a/libfdt/fdt_rw.c +++ b/libfdt/fdt_rw.c @@ -428,12 +428,14 @@ int fdt_open_into(const void *fdt, void *buf, int bufsize) if (can_assume(LATEST) || fdt_version(fdt) >= 17) { struct_size = fdt_size_dt_struct(fdt); - } else { + } else if (fdt_version(fdt) == 16) { struct_size = 0; while (fdt_next_tag(fdt, struct_size, &struct_size) != FDT_END) ; if (struct_size < 0) return struct_size; + } else { + return -FDT_ERR_BADVERSION; } if (can_assume(LIBFDT_ORDER) || diff --git a/libfdt/fdt_sw.c b/libfdt/fdt_sw.c index 68b543c..4c569ee 100644 --- a/libfdt/fdt_sw.c +++ b/libfdt/fdt_sw.c @@ -377,7 +377,7 @@ int fdt_finish(void *fdt) fdt_set_totalsize(fdt, newstroffset + fdt_size_dt_strings(fdt)); /* And fix up fields that were keeping intermediate state. */ - fdt_set_last_comp_version(fdt, FDT_FIRST_SUPPORTED_VERSION); + fdt_set_last_comp_version(fdt, FDT_LAST_COMPATIBLE_VERSION); fdt_set_magic(fdt, FDT_MAGIC); return 0; diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h index 2bc16a8..73467f7 100644 --- a/libfdt/libfdt.h +++ b/libfdt/libfdt.h @@ -14,6 +14,7 @@ extern "C" { #endif #define FDT_FIRST_SUPPORTED_VERSION 0x02 +#define FDT_LAST_COMPATIBLE_VERSION 0x10 #define FDT_LAST_SUPPORTED_VERSION 0x11 /* Error codes: informative error codes */ From 163f0469bf2ed8b2fe5aa15bc796b93c70243ddc Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 6 Jan 2021 15:26:08 +0530 Subject: [PATCH 035/104] dtc: Allow overlays to have .dtbo extension Allow the overlays to have .dtbo extension instead of just .dtb. This allows them to be identified easily by tools as well as humans. Allow the dtbo outform in dtc.c for the same. Signed-off-by: Viresh Kumar Message-Id: <30fd0e5f2156665c713cf191c5fea9a5548360c0.1609926856.git.viresh.kumar@linaro.org> Signed-off-by: David Gibson --- dtc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dtc.c b/dtc.c index bdb3f59..838c5df 100644 --- a/dtc.c +++ b/dtc.c @@ -122,6 +122,8 @@ static const char *guess_type_by_name(const char *fname, const char *fallback) return "dts"; if (!strcasecmp(s, ".yaml")) return "yaml"; + if (!strcasecmp(s, ".dtbo")) + return "dtb"; if (!strcasecmp(s, ".dtb")) return "dtb"; return fallback; @@ -357,6 +359,8 @@ int main(int argc, char *argv[]) #endif } else if (streq(outform, "dtb")) { dt_to_blob(outf, dti, outversion); + } else if (streq(outform, "dtbo")) { + dt_to_blob(outf, dti, outversion); } else if (streq(outform, "asm")) { dt_to_asm(outf, dti, outversion); } else if (streq(outform, "null")) { From 64990a272e8f7cbbdfd5a53f752b5a0db7ddf41c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacy=20Kuchci=C5=84ski?= Date: Wed, 13 Jan 2021 00:13:23 +0100 Subject: [PATCH 036/104] srcpos: increase MAX_SRCFILE_DEPTH MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some kernels require the MAX_SRCFILE_DEPTH to be bigger than 100, and since it's just a sanity check to detect infinite recursion it shouldn't hurt increasing it to 200. Signed-off-by: Ignacy Kuchciński Message-Id: Signed-off-by: David Gibson --- srcpos.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srcpos.c b/srcpos.c index f5205fb..4fdb22a 100644 --- a/srcpos.c +++ b/srcpos.c @@ -20,7 +20,7 @@ struct search_path { static struct search_path *search_path_head, **search_path_tail; /* Detect infinite include recursion. */ -#define MAX_SRCFILE_DEPTH (100) +#define MAX_SRCFILE_DEPTH (200) static int srcfile_depth; /* = 0 */ static char *get_dirname(const char *path) From ca16a723fa9dde9c5da80dba567f48715000e77c Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 6 Jan 2021 14:52:26 +1100 Subject: [PATCH 037/104] fdtdump: Fix gcc11 warning In one place, fdtdump abuses fdt_set_magic(), passing it just a small char array instead of the full fdt header it expects. That's relying on the fact that in fact fdt_set_magic() will only actually access the first 4 bytes of the buffer. This trips a new warning in GCC 11 - and it's entirely possible it was always UB. So, don't do that. Signed-off-by: David Gibson --- fdtdump.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fdtdump.c b/fdtdump.c index 9613bef..d9fb374 100644 --- a/fdtdump.c +++ b/fdtdump.c @@ -217,7 +217,7 @@ int main(int argc, char *argv[]) char *p = buf; char *endp = buf + len; - fdt_set_magic(smagic, FDT_MAGIC); + fdt32_st(smagic, FDT_MAGIC); /* poor man's memmem */ while ((endp - p) >= FDT_MAGIC_SIZE) { From 307afa1a7be8af23dee81af5c10a8a44009a768e Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 28 Jan 2021 11:24:20 +1100 Subject: [PATCH 038/104] Update Jon Loeliger's email At Jon's request update to a more current address. Signed-off-by: David Gibson --- Documentation/manual.txt | 2 +- README | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/manual.txt b/Documentation/manual.txt index adf5ccb..97e53b9 100644 --- a/Documentation/manual.txt +++ b/Documentation/manual.txt @@ -45,7 +45,7 @@ The gitweb interface for the upstream repository is: Patches should be sent to the maintainers: David Gibson - Jon Loeliger + Jon Loeliger and CCed to . 2) Description diff --git a/README b/README index 9465ee5..d9bf850 100644 --- a/README +++ b/README @@ -5,7 +5,7 @@ utility library for reading and manipulating the binary format. DTC and LIBFDT are maintained by: David Gibson -Jon Loeliger +Jon Loeliger Python library From 0db6d09584e19b1f313ebfc21c3da32a7e77ca82 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 3 Feb 2021 13:14:22 +0530 Subject: [PATCH 039/104] gitignore: Add cscope files Add cscope files in gitignore. Signed-off-by: Viresh Kumar Message-Id: <3c39a6324ef2be64f839e6e6205f4afc63486216.1612338199.git.viresh.kumar@linaro.org> Signed-off-by: David Gibson --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index 1f2a0f2..cfea8a4 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,7 @@ lex.yy.c /fdtoverlay /patches /.pc + +# cscope files +cscope.* +ncscope.* From 183df9e9c2b9e49f14f130fd7a4c0eae8f44288d Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 3 Feb 2021 15:26:36 +0530 Subject: [PATCH 040/104] gitignore: Ignore the swp files Ignore the temporary .*.swp files. Signed-off-by: Viresh Kumar Message-Id: <15496f8669ac2bb3b4b61a1a7c978947623cb7c3.1612346186.git.viresh.kumar@linaro.org> Signed-off-by: David Gibson --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index cfea8a4..8e332d8 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ *.tab.[ch] lex.yy.c *.lex.c +.*.swp /dtc /fdtdump /convert-dtsv0 From f527c867a8c6db6d787a0fc30c00415d9c59131e Mon Sep 17 00:00:00 2001 From: Jonathan Gray Date: Sat, 6 Feb 2021 21:01:10 +1100 Subject: [PATCH 041/104] util: limit gnu_printf format attribute to gcc >= 4.4.0 The gnu_printf format attribute was introduced in gcc 4.4.0 https://gcc.gnu.org/legacy-ml/gcc-help/2012-02/msg00225.html. Use the printf format attribute on earlier versions of gcc and clang (which claims to be gcc 4.2.1 in builtin defines) to fix the build with gcc 4.2.1. Fixes: 588a29f ("util: use gnu_printf format attribute") Signed-off-by: Jonathan Gray Message-Id: <20210206100110.75228-1-jsg@jsg.id.au> Signed-off-by: David Gibson --- util.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/util.h b/util.h index a771b46..c45b2c2 100644 --- a/util.h +++ b/util.h @@ -13,10 +13,10 @@ */ #ifdef __GNUC__ -#ifdef __clang__ -#define PRINTF(i, j) __attribute__((format (printf, i, j))) -#else +#if __GNUC__ >= 5 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 4) #define PRINTF(i, j) __attribute__((format (gnu_printf, i, j))) +#else +#define PRINTF(i, j) __attribute__((format (printf, i, j))) #endif #define NORETURN __attribute__((noreturn)) #else From 9d2279e7e6ee937d7c47250720c92dd58fa1aa68 Mon Sep 17 00:00:00 2001 From: Kumar Gala Date: Tue, 9 Feb 2021 12:46:41 -0600 Subject: [PATCH 042/104] checks: Change node-name check to match devicetree spec The devicetree spec limits the valid character set to: A-Z a-z 0-9 ,._+- while property can additionally have '?#'. Change the check to match the spec. Signed-off-by: Kumar Gala Message-Id: <20210209184641.63052-1-kumar.gala@linaro.org> Signed-off-by: David Gibson --- checks.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/checks.c b/checks.c index 17cb689..48e7fe9 100644 --- a/checks.c +++ b/checks.c @@ -297,7 +297,8 @@ ERROR(duplicate_property_names, check_duplicate_property_names, NULL); #define LOWERCASE "abcdefghijklmnopqrstuvwxyz" #define UPPERCASE "ABCDEFGHIJKLMNOPQRSTUVWXYZ" #define DIGITS "0123456789" -#define PROPNODECHARS LOWERCASE UPPERCASE DIGITS ",._+*#?-" +#define NODECHARS LOWERCASE UPPERCASE DIGITS ",._+-@" +#define PROPCHARS LOWERCASE UPPERCASE DIGITS ",._+*#?-" #define PROPNODECHARSSTRICT LOWERCASE UPPERCASE DIGITS ",-" static void check_node_name_chars(struct check *c, struct dt_info *dti, @@ -309,7 +310,7 @@ static void check_node_name_chars(struct check *c, struct dt_info *dti, FAIL(c, dti, node, "Bad character '%c' in node name", node->name[n]); } -ERROR(node_name_chars, check_node_name_chars, PROPNODECHARS "@"); +ERROR(node_name_chars, check_node_name_chars, NODECHARS); static void check_node_name_chars_strict(struct check *c, struct dt_info *dti, struct node *node) @@ -370,7 +371,7 @@ static void check_property_name_chars(struct check *c, struct dt_info *dti, prop->name[n]); } } -ERROR(property_name_chars, check_property_name_chars, PROPNODECHARS); +ERROR(property_name_chars, check_property_name_chars, PROPCHARS); static void check_property_name_chars_strict(struct check *c, struct dt_info *dti, From 88875268c05c9cf342d958a5c65973b0ee45888e Mon Sep 17 00:00:00 2001 From: Kumar Gala Date: Wed, 10 Feb 2021 13:39:12 -0600 Subject: [PATCH 043/104] checks: Warn on node-name and property name being the same Treat a node-name and property name at the same level of tree as a warning Signed-off-by: Kumar Gala Message-Id: <20210210193912.799544-1-kumar.gala@linaro.org> Signed-off-by: David Gibson --- checks.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/checks.c b/checks.c index 48e7fe9..c420772 100644 --- a/checks.c +++ b/checks.c @@ -331,6 +331,20 @@ static void check_node_name_format(struct check *c, struct dt_info *dti, } ERROR(node_name_format, check_node_name_format, NULL, &node_name_chars); +static void check_node_name_vs_property_name(struct check *c, + struct dt_info *dti, + struct node *node) +{ + if (!node->parent) + return; + + if (get_property(node->parent, node->name)) { + FAIL(c, dti, node, "node name and property name conflict"); + } +} +WARNING(node_name_vs_property_name, check_node_name_vs_property_name, + NULL, &node_name_chars); + static void check_unit_address_vs_reg(struct check *c, struct dt_info *dti, struct node *node) { @@ -1797,7 +1811,7 @@ WARNING(graph_endpoint, check_graph_endpoint, NULL, &graph_nodes); static struct check *check_table[] = { &duplicate_node_names, &duplicate_property_names, &node_name_chars, &node_name_format, &property_name_chars, - &name_is_string, &name_properties, + &name_is_string, &name_properties, &node_name_vs_property_name, &duplicate_label, From 8e7ff260f755548f5b039ab4e0a970dfff8073b2 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 9 Mar 2021 14:49:35 +1100 Subject: [PATCH 044/104] libfdt: Fix a possible "unchecked return value" warning Apparently the unchecked return value of the first fdt_next_tag() call in fdt_add_subnode_namelen() is tripping Coverity Scan in some circumstances, although it appears not to for the scan on our project itself. This fdt_next_tag() should always return FDT_BEGIN_NODE, since otherwise the fdt_subnode_offset_namelen() above would have returned BADOFFSET or BADSTRUCTURE. Still, add a check to shut Coverity up, gated by a can_assume() to avoid bloat in small builds. Reported-by: Ryan Long Signed-off-by: David Gibson --- libfdt/fdt_rw.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c index f13458d..2fbb545 100644 --- a/libfdt/fdt_rw.c +++ b/libfdt/fdt_rw.c @@ -349,7 +349,10 @@ int fdt_add_subnode_namelen(void *fdt, int parentoffset, return offset; /* Try to place the new node after the parent's properties */ - fdt_next_tag(fdt, parentoffset, &nextoffset); /* skip the BEGIN_NODE */ + tag = fdt_next_tag(fdt, parentoffset, &nextoffset); + /* the fdt_subnode_offset_namelen() should ensure this never hits */ + if (!can_assume(LIBFDT_FLAWLESS) && (tag != FDT_BEGIN_NODE)) + return -FDT_ERR_INTERNAL; do { offset = nextoffset; tag = fdt_next_tag(fdt, offset, &nextoffset); From 34d708249a91e0d4b89f29e7b52b21b213ce7c54 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 11 Mar 2021 18:49:55 +0900 Subject: [PATCH 045/104] dtc: Remove -O dtbo support This partially reverts 163f0469bf2e ("dtc: Allow overlays to have .dtbo extension"). I think accepting "dtbo" as --out-format is strange. This is not shown by --help, at least. *.dtb and *.dtbo should have the same format, "dtb". Signed-off-by: Masahiro Yamada Message-Id: <20210311094956.924310-1-masahiroy@kernel.org> Signed-off-by: David Gibson --- dtc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/dtc.c b/dtc.c index 838c5df..3962d3f 100644 --- a/dtc.c +++ b/dtc.c @@ -359,8 +359,6 @@ int main(int argc, char *argv[]) #endif } else if (streq(outform, "dtb")) { dt_to_blob(outf, dti, outversion); - } else if (streq(outform, "dtbo")) { - dt_to_blob(outf, dti, outversion); } else if (streq(outform, "asm")) { dt_to_asm(outf, dti, outversion); } else if (streq(outform, "null")) { From 4ca61f84dc210ae78376d992c1ce6ebe40ecb5be Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 23 Mar 2021 13:09:25 +1300 Subject: [PATCH 046/104] libfdt: Check that there is only one root node At present it is possible to have two root nodes and even access nodes in the 'second' root. Such trees should not be considered valid. This was discovered as part of a security investigation into U-Boot verified boot. Add a check for this to fdt_check_full(). Signed-off-by: Simon Glass Reported-by: Arie Haenel Reported-by: Julien Lenoir Message-Id: <20210323000926.3210733-1-sjg@chromium.org> Signed-off-by: David Gibson --- libfdt/fdt_check.c | 7 +++++++ tests/Makefile.tests | 4 +++- tests/dumptrees.c | 1 + tests/run_tests.sh | 2 +- tests/testdata.h | 1 + tests/trees.S | 19 +++++++++++++++++++ 6 files changed, 32 insertions(+), 2 deletions(-) diff --git a/libfdt/fdt_check.c b/libfdt/fdt_check.c index 9ddfdbf..13595a2 100644 --- a/libfdt/fdt_check.c +++ b/libfdt/fdt_check.c @@ -19,6 +19,7 @@ int fdt_check_full(const void *fdt, size_t bufsize) unsigned int depth = 0; const void *prop; const char *propname; + bool expect_end = false; if (bufsize < FDT_V1_SIZE) return -FDT_ERR_TRUNCATED; @@ -41,6 +42,10 @@ int fdt_check_full(const void *fdt, size_t bufsize) if (nextoffset < 0) return nextoffset; + /* If we see two root nodes, something is wrong */ + if (expect_end && tag != FDT_END) + return -FDT_ERR_BADSTRUCTURE; + switch (tag) { case FDT_NOP: break; @@ -60,6 +65,8 @@ int fdt_check_full(const void *fdt, size_t bufsize) if (depth == 0) return -FDT_ERR_BADSTRUCTURE; depth--; + if (depth == 0) + expect_end = true; break; case FDT_PROP: diff --git a/tests/Makefile.tests b/tests/Makefile.tests index cb66c9f..fe5cae8 100644 --- a/tests/Makefile.tests +++ b/tests/Makefile.tests @@ -32,7 +32,9 @@ LIB_TESTS_L = get_mem_rsv \ fs_tree1 LIB_TESTS = $(LIB_TESTS_L:%=$(TESTS_PREFIX)%) -LIBTREE_TESTS_L = truncated_property truncated_string truncated_memrsv +LIBTREE_TESTS_L = truncated_property truncated_string truncated_memrsv \ + two_roots + LIBTREE_TESTS = $(LIBTREE_TESTS_L:%=$(TESTS_PREFIX)%) DL_LIB_TESTS_L = asm_tree_dump value-labels diff --git a/tests/dumptrees.c b/tests/dumptrees.c index aecb326..02ca092 100644 --- a/tests/dumptrees.c +++ b/tests/dumptrees.c @@ -24,6 +24,7 @@ static struct { TREE(ovf_size_strings), TREE(truncated_property), TREE(truncated_string), TREE(truncated_memrsv), + TREE(two_roots) }; #define NUM_TREES (sizeof(trees) / sizeof(trees[0])) diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 4b8dada..82543fc 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -518,7 +518,7 @@ libfdt_tests () { run_test check_full $good done for bad in truncated_property.dtb truncated_string.dtb \ - truncated_memrsv.dtb; do + truncated_memrsv.dtb two_roots.dtb; do run_test check_full -n $bad done } diff --git a/tests/testdata.h b/tests/testdata.h index 0d08efb..d03f352 100644 --- a/tests/testdata.h +++ b/tests/testdata.h @@ -55,4 +55,5 @@ extern struct fdt_header bad_prop_char; extern struct fdt_header ovf_size_strings; extern struct fdt_header truncated_string; extern struct fdt_header truncated_memrsv; +extern struct fdt_header two_roots; #endif /* ! __ASSEMBLY */ diff --git a/tests/trees.S b/tests/trees.S index efab287..e2380b7 100644 --- a/tests/trees.S +++ b/tests/trees.S @@ -279,3 +279,22 @@ truncated_memrsv_rsvmap: truncated_memrsv_rsvmap_end: truncated_memrsv_end: + + + /* two root nodes */ + TREE_HDR(two_roots) + EMPTY_RSVMAP(two_roots) + +two_roots_struct: + BEGIN_NODE("") + END_NODE + BEGIN_NODE("") + END_NODE + FDTLONG(FDT_END) +two_roots_struct_end: + +two_roots_strings: +two_roots_strings_end: + +two_roots_end: + From a2def5479950d066dcf829b8c1a22874d0aea9a4 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 23 Mar 2021 14:04:10 +1300 Subject: [PATCH 047/104] libfdt: Check that the root-node name is empty The root node is supposed to have an empty name, but at present this is not checked. The behaviour of such a tree is not well defined. Most software rightly assumes that the root node is at offset 0 and does not check the name. This oddity was discovered as part of a security investigation into U-Boot verified boot. Add a check for this to fdt_check_full(). Signed-off-by: Simon Glass Reported-by: Arie Haenel Reported-by: Julien Lenoir Message-Id: <20210323010410.3222701-2-sjg@chromium.org> Signed-off-by: David Gibson --- libfdt/fdt_check.c | 10 ++++++++++ tests/Makefile.tests | 2 +- tests/dumptrees.c | 3 ++- tests/run_tests.sh | 2 +- tests/testdata.h | 1 + tests/trees.S | 15 +++++++++++++++ 6 files changed, 30 insertions(+), 3 deletions(-) diff --git a/libfdt/fdt_check.c b/libfdt/fdt_check.c index 13595a2..fa410a8 100644 --- a/libfdt/fdt_check.c +++ b/libfdt/fdt_check.c @@ -59,6 +59,16 @@ int fdt_check_full(const void *fdt, size_t bufsize) depth++; if (depth > INT_MAX) return -FDT_ERR_BADSTRUCTURE; + + /* The root node must have an empty name */ + if (depth == 1) { + const char *name; + int len; + + name = fdt_get_name(fdt, offset, &len); + if (*name || len) + return -FDT_ERR_BADSTRUCTURE; + } break; case FDT_END_NODE: diff --git a/tests/Makefile.tests b/tests/Makefile.tests index fe5cae8..2b47627 100644 --- a/tests/Makefile.tests +++ b/tests/Makefile.tests @@ -33,7 +33,7 @@ LIB_TESTS_L = get_mem_rsv \ LIB_TESTS = $(LIB_TESTS_L:%=$(TESTS_PREFIX)%) LIBTREE_TESTS_L = truncated_property truncated_string truncated_memrsv \ - two_roots + two_roots named_root LIBTREE_TESTS = $(LIBTREE_TESTS_L:%=$(TESTS_PREFIX)%) diff --git a/tests/dumptrees.c b/tests/dumptrees.c index 02ca092..f1e0ea9 100644 --- a/tests/dumptrees.c +++ b/tests/dumptrees.c @@ -24,7 +24,8 @@ static struct { TREE(ovf_size_strings), TREE(truncated_property), TREE(truncated_string), TREE(truncated_memrsv), - TREE(two_roots) + TREE(two_roots), + TREE(named_root) }; #define NUM_TREES (sizeof(trees) / sizeof(trees[0])) diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 82543fc..5c2b1e8 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -518,7 +518,7 @@ libfdt_tests () { run_test check_full $good done for bad in truncated_property.dtb truncated_string.dtb \ - truncated_memrsv.dtb two_roots.dtb; do + truncated_memrsv.dtb two_roots.dtb named_root.dtb; do run_test check_full -n $bad done } diff --git a/tests/testdata.h b/tests/testdata.h index d03f352..4f9e3ba 100644 --- a/tests/testdata.h +++ b/tests/testdata.h @@ -56,4 +56,5 @@ extern struct fdt_header ovf_size_strings; extern struct fdt_header truncated_string; extern struct fdt_header truncated_memrsv; extern struct fdt_header two_roots; +extern struct fdt_header named_root; #endif /* ! __ASSEMBLY */ diff --git a/tests/trees.S b/tests/trees.S index e2380b7..95d599d 100644 --- a/tests/trees.S +++ b/tests/trees.S @@ -298,3 +298,18 @@ two_roots_strings_end: two_roots_end: + + /* root node with a non-empty name */ + TREE_HDR(named_root) + EMPTY_RSVMAP(named_root) + +named_root_struct: + BEGIN_NODE("fake") + END_NODE + FDTLONG(FDT_END) +named_root_struct_end: + +named_root_strings: +named_root_strings_end: + +named_root_end: From b07b62ee3342db3af74a8bd056dce810764cc7f1 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Tue, 6 Apr 2021 14:07:12 -0500 Subject: [PATCH 048/104] libfdt: Add FDT alignment check to fdt_check_header() Only checking the FDT alignment in fdt_ro_probe_() means that fdt_check_header() can pass, but then subsequent API calls fail on alignment checks. Let's add an alignment check to fdt_check_header() so alignment errors are found up front. Cc: Tom Rini Cc: Frank Rowand Signed-off-by: Rob Herring Message-Id: <20210406190712.2118098-1-robh@kernel.org> Signed-off-by: David Gibson --- libfdt/fdt.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libfdt/fdt.c b/libfdt/fdt.c index 3e89307..9fe7cf4 100644 --- a/libfdt/fdt.c +++ b/libfdt/fdt.c @@ -90,6 +90,10 @@ int fdt_check_header(const void *fdt) { size_t hdrsize; + /* The device tree must be at an 8-byte aligned address */ + if ((uintptr_t)fdt & 7) + return -FDT_ERR_ALIGNMENT; + if (fdt_magic(fdt) != FDT_MAGIC) return -FDT_ERR_BADMAGIC; if (!can_assume(LATEST)) { From 9bb9b8d0b4a03d119162221cbdfd91354653d902 Mon Sep 17 00:00:00 2001 From: Ilya Lipnitskiy Date: Mon, 3 May 2021 20:59:41 -0700 Subject: [PATCH 049/104] checks: tigthen up nr-gpios prop exception There are no instances of nr-gpio in the Linux kernel tree, only "[,]nr-gpios", so make the check stricter. nr-gpios without a "vendor," prefix is also invalid, according to the DT spec[0], and there are no DT files in the Linux kernel tree with non-vendor nr-gpios. There are some drivers, but they are not DT spec compliant, so don't suppress the check for them. [0]: Link: https://github.com/devicetree-org/dt-schema/blob/cb53a16a1eb3e2169ce170c071e47940845ec26e/schemas/gpio/gpio-consumer.yaml#L20 Signed-off-by: Ilya Lipnitskiy Cc: Rob Herring Message-Id: <20210504035944.8453-2-ilya.lipnitskiy@gmail.com> Signed-off-by: David Gibson --- checks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checks.c b/checks.c index c420772..7e9d73a 100644 --- a/checks.c +++ b/checks.c @@ -1494,7 +1494,7 @@ static bool prop_is_gpio(struct property *prop) * *-gpios and *-gpio can appear in property names, * so skip over any false matches (only one known ATM) */ - if (strstr(prop->name, "nr-gpio")) + if (strstr(prop->name, ",nr-gpios")) return false; str = strrchr(prop->name, '-'); From 09c6a6e88718de6b2ef646a015187605caa5789f Mon Sep 17 00:00:00 2001 From: Ilya Lipnitskiy Date: Mon, 3 May 2021 20:59:42 -0700 Subject: [PATCH 050/104] dtc.h: add strends for suffix matching Logic is similar to strcmp_suffix in /drivers/of/property.c with the exception that strends allows string length to equal suffix length. Signed-off-by: Ilya Lipnitskiy Message-Id: <20210504035944.8453-3-ilya.lipnitskiy@gmail.com> Signed-off-by: David Gibson --- dtc.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dtc.h b/dtc.h index d3e82fb..6296361 100644 --- a/dtc.h +++ b/dtc.h @@ -86,6 +86,16 @@ static inline uint64_t dtb_ld64(const void *p) #define streq(a, b) (strcmp((a), (b)) == 0) #define strstarts(s, prefix) (strncmp((s), (prefix), strlen(prefix)) == 0) #define strprefixeq(a, n, b) (strlen(b) == (n) && (memcmp(a, b, n) == 0)) +static inline bool strends(const char *str, const char *suffix) +{ + unsigned int len, suffix_len; + + len = strlen(str); + suffix_len = strlen(suffix); + if (len < suffix_len) + return false; + return streq(str + len - suffix_len, suffix); +} #define ALIGN(x, a) (((x) + (a) - 1) & ~((a) - 1)) From ad4abfadb687060d047a9cff664b471c7effb3b1 Mon Sep 17 00:00:00 2001 From: Ilya Lipnitskiy Date: Mon, 3 May 2021 20:59:43 -0700 Subject: [PATCH 051/104] checks: replace strstr and strrchr with strends Makes the logic more clear Signed-off-by: Ilya Lipnitskiy Message-Id: <20210504035944.8453-4-ilya.lipnitskiy@gmail.com> Signed-off-by: David Gibson --- checks.c | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/checks.c b/checks.c index 7e9d73a..eb91c8e 100644 --- a/checks.c +++ b/checks.c @@ -687,8 +687,7 @@ static void check_names_is_string_list(struct check *c, struct dt_info *dti, struct property *prop; for_each_property(node, prop) { - const char *s = strrchr(prop->name, '-'); - if (!s || !streq(s, "-names")) + if (!strends(prop->name, "-names")) continue; c->data = prop->name; @@ -1488,24 +1487,17 @@ WARNING_PROPERTY_PHANDLE_CELLS(thermal_sensors, "thermal-sensors", "#thermal-sen static bool prop_is_gpio(struct property *prop) { - char *str; - /* * *-gpios and *-gpio can appear in property names, * so skip over any false matches (only one known ATM) */ - if (strstr(prop->name, ",nr-gpios")) + if (strends(prop->name, ",nr-gpios")) return false; - str = strrchr(prop->name, '-'); - if (str) - str++; - else - str = prop->name; - if (!(streq(str, "gpios") || streq(str, "gpio"))) - return false; - - return true; + return strends(prop->name, "-gpios") || + streq(prop->name, "gpios") || + strends(prop->name, "-gpio") || + streq(prop->name, "gpio"); } static void check_gpios_property(struct check *c, @@ -1540,13 +1532,10 @@ static void check_deprecated_gpio_property(struct check *c, struct property *prop; for_each_property(node, prop) { - char *str; - if (!prop_is_gpio(prop)) continue; - str = strstr(prop->name, "gpio"); - if (!streq(str, "gpio")) + if (!strends(prop->name, "gpio")) continue; FAIL_PROP(c, dti, node, prop, From c8bddd1060957ed0a5c95ea364cc13891742b09f Mon Sep 17 00:00:00 2001 From: Ilya Lipnitskiy Date: Mon, 3 May 2021 20:59:44 -0700 Subject: [PATCH 052/104] tests: add a positive gpio test case Ensure that properly named properties don't trigger warnings Signed-off-by: Ilya Lipnitskiy Message-Id: <20210504035944.8453-5-ilya.lipnitskiy@gmail.com> Signed-off-by: David Gibson --- tests/good-gpio.dts | 12 ++++++++++++ tests/run_tests.sh | 2 ++ 2 files changed, 14 insertions(+) create mode 100644 tests/good-gpio.dts diff --git a/tests/good-gpio.dts b/tests/good-gpio.dts new file mode 100644 index 0000000..65d1c17 --- /dev/null +++ b/tests/good-gpio.dts @@ -0,0 +1,12 @@ +/dts-v1/; + +/ { + gpio: gpio-controller { + #gpio-cells = <3>; + }; + + node { + foo,nr-gpios = <1>; + foo-gpios = <&gpio 1 2 3>; + }; +}; diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 5c2b1e8..0e8ecdb 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -709,10 +709,12 @@ dtc_tests () { check_tests "$SRCDIR/unit-addr-unique.dts" unique_unit_address check_tests "$SRCDIR/bad-phandle-cells.dts" interrupts_extended_property check_tests "$SRCDIR/bad-gpio.dts" gpios_property + check_tests "$SRCDIR/good-gpio.dts" -n gpios_property check_tests "$SRCDIR/bad-graph.dts" graph_child_address check_tests "$SRCDIR/bad-graph.dts" graph_port check_tests "$SRCDIR/bad-graph.dts" graph_endpoint run_sh_test "$SRCDIR/dtc-checkfails.sh" deprecated_gpio_property -- -Wdeprecated_gpio_property -I dts -O dtb "$SRCDIR/bad-gpio.dts" + run_sh_test "$SRCDIR/dtc-checkfails.sh" -n deprecated_gpio_property -- -Wdeprecated_gpio_property -I dts -O dtb "$SRCDIR/good-gpio.dts" check_tests "$SRCDIR/bad-interrupt-cells.dts" interrupts_property check_tests "$SRCDIR/bad-interrupt-controller.dts" interrupt_provider run_sh_test "$SRCDIR/dtc-checkfails.sh" node_name_chars -- -I dtb -O dtb bad_node_char.dtb From 61e513439e408035b4da9ce7120a91297371a308 Mon Sep 17 00:00:00 2001 From: Tom Rini Date: Mon, 24 May 2021 11:49:10 -0400 Subject: [PATCH 053/104] pylibfdt: Rework "avoid unused variable warning" lines Clang has -Wself-assign enabled by default under -Wall and so when building with -Werror we would get an error here. Inspired by Linux kernel git commit a21151b9d81a ("tools/build: tweak unused value workaround") make use of the fact that both Clang and GCC support casting to `void` as the method to note that something is intentionally unused. Signed-off-by: Tom Rini Message-Id: <20210524154910.30523-1-trini@konsulko.com> Signed-off-by: David Gibson --- pylibfdt/libfdt.i | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i index 3e09d3a..51ee801 100644 --- a/pylibfdt/libfdt.i +++ b/pylibfdt/libfdt.i @@ -1009,7 +1009,7 @@ typedef uint32_t fdt32_t; } $1 = (void *)PyByteArray_AsString($input); fdt = $1; - fdt = fdt; /* avoid unused variable warning */ + (void)fdt; /* avoid unused variable warning */ } /* Some functions do change the device tree, so use void * */ @@ -1020,7 +1020,7 @@ typedef uint32_t fdt32_t; } $1 = PyByteArray_AsString($input); fdt = $1; - fdt = fdt; /* avoid unused variable warning */ + (void)fdt; /* avoid unused variable warning */ } /* typemap used for fdt_get_property_by_offset() */ From 2dffc192a77fda9d9eeb393894e686c98246e8f3 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Tue, 25 May 2021 20:03:31 -0500 Subject: [PATCH 054/104] yamltree: Remove marker ordering dependency The check for phandle markers is fragile because the phandle marker must be after a type marker. The only guarantee for markers is they are in offset order. The order at a specific offset is undefined. Rework yaml_propval_int() to get the full marker list, so it can find a phandle marker no matter the ordering. Signed-off-by: Rob Herring Message-Id: <20210526010335.860787-2-robh@kernel.org> Signed-off-by: David Gibson --- yamltree.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/yamltree.c b/yamltree.c index e63d32f..55908c8 100644 --- a/yamltree.c +++ b/yamltree.c @@ -29,11 +29,12 @@ char *yaml_error_name[] = { (emitter)->problem, __func__, __LINE__); \ }) -static void yaml_propval_int(yaml_emitter_t *emitter, struct marker *markers, char *data, unsigned int len, int width) +static void yaml_propval_int(yaml_emitter_t *emitter, struct marker *markers, + char *data, unsigned int seq_offset, unsigned int len, int width) { yaml_event_t event; void *tag; - unsigned int off, start_offset = markers->offset; + unsigned int off; switch(width) { case 1: tag = "!u8"; break; @@ -66,7 +67,7 @@ static void yaml_propval_int(yaml_emitter_t *emitter, struct marker *markers, ch m = markers; is_phandle = false; for_each_marker_of_type(m, REF_PHANDLE) { - if (m->offset == (start_offset + off)) { + if (m->offset == (seq_offset + off)) { is_phandle = true; break; } @@ -114,6 +115,7 @@ static void yaml_propval(yaml_emitter_t *emitter, struct property *prop) yaml_event_t event; unsigned int len = prop->val.len; struct marker *m = prop->val.markers; + struct marker *markers = prop->val.markers; /* Emit the property name */ yaml_scalar_event_initialize(&event, NULL, @@ -151,19 +153,19 @@ static void yaml_propval(yaml_emitter_t *emitter, struct property *prop) switch(m->type) { case TYPE_UINT16: - yaml_propval_int(emitter, m, data, chunk_len, 2); + yaml_propval_int(emitter, markers, data, m->offset, chunk_len, 2); break; case TYPE_UINT32: - yaml_propval_int(emitter, m, data, chunk_len, 4); + yaml_propval_int(emitter, markers, data, m->offset, chunk_len, 4); break; case TYPE_UINT64: - yaml_propval_int(emitter, m, data, chunk_len, 8); + yaml_propval_int(emitter, markers, data, m->offset, chunk_len, 8); break; case TYPE_STRING: yaml_propval_string(emitter, data, chunk_len); break; default: - yaml_propval_int(emitter, m, data, chunk_len, 1); + yaml_propval_int(emitter, markers, data, m->offset, chunk_len, 1); break; } } From 6b3081abc4ac6b0b084ebb8a09e76854dffa797c Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Tue, 25 May 2021 20:03:32 -0500 Subject: [PATCH 055/104] checks: Add check_is_cell() for all phandle+arg properties There's already a check for '#.*-cells' properties, so let's enable it for all the ones we already know about. Signed-off-by: Rob Herring Message-Id: <20210526010335.860787-3-robh@kernel.org> Signed-off-by: David Gibson --- checks.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/checks.c b/checks.c index eb91c8e..6a01a24 100644 --- a/checks.c +++ b/checks.c @@ -1466,7 +1466,8 @@ static void check_provider_cells_property(struct check *c, } #define WARNING_PROPERTY_PHANDLE_CELLS(nm, propname, cells_name, ...) \ static struct provider nm##_provider = { (propname), (cells_name), __VA_ARGS__ }; \ - WARNING(nm##_property, check_provider_cells_property, &nm##_provider, &phandle_references); + WARNING_IF_NOT_CELL(nm##_is_cell, cells_name); \ + WARNING(nm##_property, check_provider_cells_property, &nm##_provider, &nm##_is_cell, &phandle_references); WARNING_PROPERTY_PHANDLE_CELLS(clocks, "clocks", "#clock-cells"); WARNING_PROPERTY_PHANDLE_CELLS(cooling_device, "cooling-device", "#cooling-cells"); @@ -1843,21 +1844,37 @@ static struct check *check_table[] = { &chosen_node_is_root, &chosen_node_bootargs, &chosen_node_stdout_path, &clocks_property, + &clocks_is_cell, &cooling_device_property, + &cooling_device_is_cell, &dmas_property, + &dmas_is_cell, &hwlocks_property, + &hwlocks_is_cell, &interrupts_extended_property, + &interrupts_extended_is_cell, &io_channels_property, + &io_channels_is_cell, &iommus_property, + &iommus_is_cell, &mboxes_property, + &mboxes_is_cell, &msi_parent_property, + &msi_parent_is_cell, &mux_controls_property, + &mux_controls_is_cell, &phys_property, + &phys_is_cell, &power_domains_property, + &power_domains_is_cell, &pwms_property, + &pwms_is_cell, &resets_property, + &resets_is_cell, &sound_dai_property, + &sound_dai_is_cell, &thermal_sensors_property, + &thermal_sensors_is_cell, &deprecated_gpio_property, &gpios_property, From 0c3fd9b6acebc980e5e881385afd265fa7665270 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Tue, 25 May 2021 20:03:33 -0500 Subject: [PATCH 056/104] checks: Drop interrupt_cells_is_cell check With the prior commit, this check is now redundant. Signed-off-by: Rob Herring Message-Id: <20210526010335.860787-4-robh@kernel.org> Signed-off-by: David Gibson --- checks.c | 3 +-- tests/run_tests.sh | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/checks.c b/checks.c index 6a01a24..7ff044d 100644 --- a/checks.c +++ b/checks.c @@ -672,7 +672,6 @@ ERROR(omit_unused_nodes, fixup_omit_unused_nodes, NULL, &phandle_references, &pa */ WARNING_IF_NOT_CELL(address_cells_is_cell, "#address-cells"); WARNING_IF_NOT_CELL(size_cells_is_cell, "#size-cells"); -WARNING_IF_NOT_CELL(interrupt_cells_is_cell, "#interrupt-cells"); WARNING_IF_NOT_STRING(device_type_is_string, "device_type"); WARNING_IF_NOT_STRING(model_is_string, "model"); @@ -1809,7 +1808,7 @@ static struct check *check_table[] = { &phandle_references, &path_references, &omit_unused_nodes, - &address_cells_is_cell, &size_cells_is_cell, &interrupt_cells_is_cell, + &address_cells_is_cell, &size_cells_is_cell, &device_type_is_string, &model_is_string, &status_is_string, &label_is_string, diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 0e8ecdb..0e270fe 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -691,7 +691,7 @@ dtc_tests () { run_sh_test "$SRCDIR/dtc-fatal.sh" -I dts -O dtb "$SRCDIR/nonexist-node-ref2.dts" check_tests "$SRCDIR/bad-name-property.dts" name_properties - check_tests "$SRCDIR/bad-ncells.dts" address_cells_is_cell size_cells_is_cell interrupt_cells_is_cell + check_tests "$SRCDIR/bad-ncells.dts" address_cells_is_cell size_cells_is_cell interrupts_extended_is_cell check_tests "$SRCDIR/bad-string-props.dts" device_type_is_string model_is_string status_is_string label_is_string compatible_is_string_list names_is_string_list check_tests "$SRCDIR/bad-chosen.dts" chosen_node_is_root check_tests "$SRCDIR/bad-chosen.dts" chosen_node_bootargs @@ -740,7 +740,7 @@ dtc_tests () { # Check warning options - run_sh_test "$SRCDIR/dtc-checkfails.sh" address_cells_is_cell interrupt_cells_is_cell -n size_cells_is_cell -- -Wno_size_cells_is_cell -I dts -O dtb "$SRCDIR/bad-ncells.dts" + run_sh_test "$SRCDIR/dtc-checkfails.sh" address_cells_is_cell interrupts_extended_is_cell -n size_cells_is_cell -- -Wno_size_cells_is_cell -I dts -O dtb "$SRCDIR/bad-ncells.dts" run_sh_test "$SRCDIR/dtc-fails.sh" -n test-warn-output.test.dtb -I dts -O dtb "$SRCDIR/bad-ncells.dts" run_sh_test "$SRCDIR/dtc-fails.sh" test-error-output.test.dtb -I dts -O dtb bad-ncells.dts -Esize_cells_is_cell run_sh_test "$SRCDIR/dtc-checkfails.sh" always_fail -- -Walways_fail -I dts -O dtb "$SRCDIR/test_tree1.dts" From e59ca36fb70e32f8b4850135ab94fdf44cd22557 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 8 Jun 2021 14:15:25 +1000 Subject: [PATCH 057/104] Make handling of cpp line information more tolerant At least some cpp implementations, in at least some circumstances place multiple numbers after the file name when they put line number information into the output. We don't really care what the content of these is, but we want the dtc lexer not to choke on this, so adjust the rule for handling cpp line number information accordingly. Signed-off-by: David Gibson --- dtc-lexer.l | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dtc-lexer.l b/dtc-lexer.l index b3b7270..5568b4a 100644 --- a/dtc-lexer.l +++ b/dtc-lexer.l @@ -57,7 +57,7 @@ static void PRINTF(1, 2) lexical_error(const char *fmt, ...); push_input_file(name); } -<*>^"#"(line)?[ \t]+[0-9]+[ \t]+{STRING}([ \t]+[0-9]+)? { +<*>^"#"(line)?[ \t]+[0-9]+[ \t]+{STRING}([ \t]+[0-9]+)* { char *line, *fnstart, *fnend; struct data fn; /* skip text before line # */ From 4c2ef8f4d14ca411b41b9c6891028d58d2079bf7 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 8 Jun 2021 15:17:11 +1000 Subject: [PATCH 058/104] checks: Introduce is_multiple_of() In a number of places we check if one number is a multiple of another, using a modulus. In some of those cases the divisor is potentially zero, which needs special handling or we could trigger a divide by zero. Introduce an is_multiple_of() helper to safely handle this case, and use it in a bunch of places. This should close Coverity issue 1501687, maybe others as well. Signed-off-by: David Gibson --- checks.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/checks.c b/checks.c index 7ff044d..e6c7c3e 100644 --- a/checks.c +++ b/checks.c @@ -143,6 +143,14 @@ static void check_nodes_props(struct check *c, struct dt_info *dti, struct node check_nodes_props(c, dti, child); } +static bool is_multiple_of(int multiple, int divisor) +{ + if (divisor == 0) + return multiple == 0; + else + return (multiple % divisor) == 0; +} + static bool run_check(struct check *c, struct dt_info *dti) { struct node *dt = dti->dt; @@ -766,7 +774,7 @@ static void check_reg_format(struct check *c, struct dt_info *dti, size_cells = node_size_cells(node->parent); entrylen = (addr_cells + size_cells) * sizeof(cell_t); - if (!entrylen || (prop->val.len % entrylen) != 0) + if (!is_multiple_of(prop->val.len, entrylen)) FAIL_PROP(c, dti, node, prop, "property has invalid length (%d bytes) " "(#address-cells == %d, #size-cells == %d)", prop->val.len, addr_cells, size_cells); @@ -807,7 +815,7 @@ static void check_ranges_format(struct check *c, struct dt_info *dti, "#size-cells (%d) differs from %s (%d)", ranges, c_size_cells, node->parent->fullpath, p_size_cells); - } else if ((prop->val.len % entrylen) != 0) { + } else if (!is_multiple_of(prop->val.len, entrylen)) { FAIL_PROP(c, dti, node, prop, "\"%s\" property has invalid length (%d bytes) " "(parent #address-cells == %d, child #address-cells == %d, " "#size-cells == %d)", ranges, prop->val.len, @@ -1382,7 +1390,7 @@ static void check_property_phandle_args(struct check *c, struct node *root = dti->dt; int cell, cellsize = 0; - if (prop->val.len % sizeof(cell_t)) { + if (!is_multiple_of(prop->val.len, sizeof(cell_t))) { FAIL_PROP(c, dti, node, prop, "property size (%d) is invalid, expected multiple of %zu", prop->val.len, sizeof(cell_t)); @@ -1594,7 +1602,7 @@ static void check_interrupts_property(struct check *c, if (!irq_prop) return; - if (irq_prop->val.len % sizeof(cell_t)) + if (!is_multiple_of(irq_prop->val.len, sizeof(cell_t))) FAIL_PROP(c, dti, node, irq_prop, "size (%d) is invalid, expected multiple of %zu", irq_prop->val.len, sizeof(cell_t)); @@ -1643,7 +1651,7 @@ static void check_interrupts_property(struct check *c, } irq_cells = propval_cell(prop); - if (irq_prop->val.len % (irq_cells * sizeof(cell_t))) { + if (!is_multiple_of(irq_prop->val.len, irq_cells * sizeof(cell_t))) { FAIL_PROP(c, dti, node, prop, "size is (%d), expected multiple of %d", irq_prop->val.len, (int)(irq_cells * sizeof(cell_t))); From 21d61d18f9682de69d7cc1632003fb4e476af43a Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 8 Jun 2021 15:28:55 +1000 Subject: [PATCH 059/104] Fix CID 1461557 Coverity gets a bit confused by loading fdt_size_dt_strings() and using it in a memmove(). In fact this is safe because the callers have verified this information (via FDT_RW_PROBE() in fdt_pack() or construction in fdt_open_into()). Passing in strings_size like we already do struct_size seems to get Coverity to follow what's going on here. Signed-off-by: David Gibson --- libfdt/fdt_rw.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c index 2fbb545..3621d36 100644 --- a/libfdt/fdt_rw.c +++ b/libfdt/fdt_rw.c @@ -394,7 +394,9 @@ int fdt_del_node(void *fdt, int nodeoffset) } static void fdt_packblocks_(const char *old, char *new, - int mem_rsv_size, int struct_size) + int mem_rsv_size, + int struct_size, + int strings_size) { int mem_rsv_off, struct_off, strings_off; @@ -409,8 +411,7 @@ static void fdt_packblocks_(const char *old, char *new, fdt_set_off_dt_struct(new, struct_off); fdt_set_size_dt_struct(new, struct_size); - memmove(new + strings_off, old + fdt_off_dt_strings(old), - fdt_size_dt_strings(old)); + memmove(new + strings_off, old + fdt_off_dt_strings(old), strings_size); fdt_set_off_dt_strings(new, strings_off); fdt_set_size_dt_strings(new, fdt_size_dt_strings(old)); } @@ -470,7 +471,8 @@ int fdt_open_into(const void *fdt, void *buf, int bufsize) return -FDT_ERR_NOSPACE; } - fdt_packblocks_(fdt, tmp, mem_rsv_size, struct_size); + fdt_packblocks_(fdt, tmp, mem_rsv_size, struct_size, + fdt_size_dt_strings(fdt)); memmove(buf, tmp, newsize); fdt_set_magic(buf, FDT_MAGIC); @@ -490,7 +492,8 @@ int fdt_pack(void *fdt) mem_rsv_size = (fdt_num_mem_rsv(fdt)+1) * sizeof(struct fdt_reserve_entry); - fdt_packblocks_(fdt, fdt, mem_rsv_size, fdt_size_dt_struct(fdt)); + fdt_packblocks_(fdt, fdt, mem_rsv_size, fdt_size_dt_struct(fdt), + fdt_size_dt_strings(fdt)); fdt_set_totalsize(fdt, fdt_data_size_(fdt)); return 0; From b6910bec11614980a21e46fbccc35934b671bd81 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 8 Jun 2021 17:00:49 +1000 Subject: [PATCH 060/104] Bump version to v1.6.1 A number of fixes have accumulated, so rolling up another release. Signed-off-by: David Gibson --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index c187d5f..ea8c659 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ # VERSION = 1 PATCHLEVEL = 6 -SUBLEVEL = 0 +SUBLEVEL = 1 EXTRAVERSION = LOCAL_VERSION = CONFIG_LOCALVERSION = From 24e7f511fd4acaf48d25374f88dbdbdb277e6a26 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 11 Jun 2021 18:10:34 +0100 Subject: [PATCH 061/104] fdtdump: Fix signedness comparisons warnings With -Wsign-compare, compilers warn about a mismatching signedness in comparisons in fdtdump.c. The "len" parameter to valid_header() refers to a memory size, not a file offset, so the (unsigned) size_t is better fit, and fixes the warning nicely. In the main function we compare the difference between two pointers, which produces a signed ptrdiff_t type. However the while loop above the comparison makes sure that "p" always points before "endp" (by virtue of the limit in the memchr() call). This means "endp - p" is never negative, so we can safely cast this expression to an unsigned type. This fixes "make fdtdump", when compiled with -Wsign-compare. Signed-off-by: Andre Przywara Message-Id: <20210611171040.25524-3-andre.przywara@arm.com> Signed-off-by: David Gibson --- fdtdump.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fdtdump.c b/fdtdump.c index d9fb374..bdc0f94 100644 --- a/fdtdump.c +++ b/fdtdump.c @@ -18,7 +18,7 @@ #include "util.h" #define FDT_MAGIC_SIZE 4 -#define MAX_VERSION 17 +#define MAX_VERSION 17U #define ALIGN(x, a) (((x) + ((a) - 1)) & ~((a) - 1)) #define PALIGN(p, a) ((void *)(ALIGN((unsigned long)(p), (a)))) @@ -163,7 +163,7 @@ static const char * const usage_opts_help[] = { USAGE_COMMON_OPTS_HELP }; -static bool valid_header(char *p, off_t len) +static bool valid_header(char *p, size_t len) { if (len < sizeof(struct fdt_header) || fdt_magic(p) != FDT_MAGIC || @@ -235,7 +235,7 @@ int main(int argc, char *argv[]) } ++p; } - if (!p || endp - p < sizeof(struct fdt_header)) + if (!p || (size_t)(endp - p) < sizeof(struct fdt_header)) die("%s: could not locate fdt magic\n", file); printf("%s: found fdt at offset %#tx\n", file, p - buf); buf = p; From 5bec74a6d13519381a40b5433ede7849a75a8d79 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 11 Jun 2021 18:10:37 +0100 Subject: [PATCH 062/104] dtc: Fix signedness comparisons warnings: reservednum With -Wsign-compare, compilers warn about a mismatching signedness in comparisons in code using the "reservednum" variable. There is obviously little sense in having a negative number of reserved memory entries, so let's make this variable and all its users unsigned. Signed-off-by: Andre Przywara Message-Id: <20210611171040.25524-6-andre.przywara@arm.com> Signed-off-by: David Gibson --- dtc.c | 4 ++-- dtc.h | 2 +- flattree.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dtc.c b/dtc.c index 3962d3f..bc786c5 100644 --- a/dtc.c +++ b/dtc.c @@ -12,7 +12,7 @@ * Command line options */ int quiet; /* Level of quietness */ -int reservenum; /* Number of memory reservation slots */ +unsigned int reservenum;/* Number of memory reservation slots */ int minsize; /* Minimum blob size */ int padsize; /* Additional padding to blob */ int alignsize; /* Additional padding to blob accroding to the alignsize */ @@ -197,7 +197,7 @@ int main(int argc, char *argv[]) depname = optarg; break; case 'R': - reservenum = strtol(optarg, NULL, 0); + reservenum = strtoul(optarg, NULL, 0); break; case 'S': minsize = strtol(optarg, NULL, 0); diff --git a/dtc.h b/dtc.h index 6296361..47664f2 100644 --- a/dtc.h +++ b/dtc.h @@ -35,7 +35,7 @@ * Command line options */ extern int quiet; /* Level of quietness */ -extern int reservenum; /* Number of memory reservation slots */ +extern unsigned int reservenum; /* Number of memory reservation slots */ extern int minsize; /* Minimum blob size */ extern int padsize; /* Additional padding to blob */ extern int alignsize; /* Additional padding to blob accroding to the alignsize */ diff --git a/flattree.c b/flattree.c index 4659afb..3d0204f 100644 --- a/flattree.c +++ b/flattree.c @@ -295,7 +295,7 @@ static struct data flatten_reserve_list(struct reserve_info *reservelist, { struct reserve_info *re; struct data d = empty_data; - int j; + unsigned int j; for (re = reservelist; re; re = re->next) { d = data_append_re(d, re->address, re->size); From ecfb438c07fa468a129e81c7d84c7c293c7b0150 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 11 Jun 2021 18:10:38 +0100 Subject: [PATCH 063/104] dtc: Fix signedness comparisons warnings: pointer diff With -Wsign-compare, compilers warn about a mismatching signedness in comparisons in the function get_node_by_path(). Taking the difference between two pointers results in a signed ptrdiff_t type, which mismatches the unsigned type returned by strlen(). Since "p" has been returned by a call to strchr() with "path" as its argument, we know for sure that it's bigger than "path", so the difference must be positive. So a cast to an unsigned type is valid. Signed-off-by: Andre Przywara Message-Id: <20210611171040.25524-7-andre.przywara@arm.com> Signed-off-by: David Gibson --- livetree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/livetree.c b/livetree.c index 7eacd02..dfbae65 100644 --- a/livetree.c +++ b/livetree.c @@ -526,7 +526,7 @@ struct node *get_node_by_path(struct node *tree, const char *path) p = strchr(path, '/'); for_each_child(tree, child) { - if (p && strprefixeq(path, p - path, child->name)) + if (p && strprefixeq(path, (size_t)(p - path), child->name)) return get_node_by_path(child, p+1); else if (!p && streq(path, child->name)) return child; From d966f08fcd21ded2ed6608097e9832e9466857fd Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 18 Jun 2021 18:20:26 +0100 Subject: [PATCH 064/104] tests: Fix signedness comparisons warnings With -Wsign-compare, compilers warn about a mismatching signedness in comparisons in various files in the tests/ directory. For about half of the cases we can simply change the signed variable to be of an unsigned type, because they will never need to store negative values (which is the best fix of the problem). In the remaining cases we can cast the signed variable to an unsigned type, provided we know for sure it is not negative. We see two different scenarios here: - We either just explicitly checked for this variable to be positive (if (rc < 0) FAIL();), or - We rely on a function returning only positive values in the "length" pointer if the function returned successfully: which we just checked. At two occassions we compare with a constant "-1" (even though the variable is unsigned), so we just change this to ~0U to create an unsigned comparison value. Since this is about the tests, let's also add explicit tests for those values really not being negative. This fixes "make tests" (but not "make check" yet), when compiled with -Wsign-compare. Signed-off-by: Andre Przywara Message-Id: <20210618172030.9684-2-andre.przywara@arm.com> Signed-off-by: David Gibson --- tests/dumptrees.c | 2 +- tests/fs_tree1.c | 2 +- tests/get_name.c | 4 +++- tests/integer-expressions.c | 2 +- tests/nopulate.c | 3 ++- tests/overlay.c | 6 +++++- tests/phandle_format.c | 2 +- tests/property_iterate.c | 2 +- tests/references.c | 2 +- tests/set_name.c | 10 ++++++++-- tests/subnode_iterate.c | 2 +- tests/tests.h | 2 +- tests/testutils.c | 12 +++++++++--- 13 files changed, 35 insertions(+), 16 deletions(-) diff --git a/tests/dumptrees.c b/tests/dumptrees.c index f1e0ea9..08967b3 100644 --- a/tests/dumptrees.c +++ b/tests/dumptrees.c @@ -32,7 +32,7 @@ static struct { int main(int argc, char *argv[]) { - int i; + unsigned int i; if (argc != 2) { fprintf(stderr, "Missing output directory argument\n"); diff --git a/tests/fs_tree1.c b/tests/fs_tree1.c index dff3880..978f6a3 100644 --- a/tests/fs_tree1.c +++ b/tests/fs_tree1.c @@ -54,7 +54,7 @@ static void mkfile(const char *name, void *data, size_t len) rc = write(fd, data, len); if (rc < 0) FAIL("write(\"%s\"): %s", name, strerror(errno)); - if (rc != len) + if ((unsigned)rc != len) FAIL("write(\"%s\"): short write", name); rc = close(fd); diff --git a/tests/get_name.c b/tests/get_name.c index 5a35103..d20bf30 100644 --- a/tests/get_name.c +++ b/tests/get_name.c @@ -34,12 +34,14 @@ static void check_name(void *fdt, const char *path) offset, getname, len); if (!getname) FAIL("fdt_get_name(%d): %s", offset, fdt_strerror(len)); + if (len < 0) + FAIL("negative name length (%d) for returned node name\n", len); if (strcmp(getname, checkname) != 0) FAIL("fdt_get_name(%s) returned \"%s\" instead of \"%s\"", path, getname, checkname); - if (len != strlen(getname)) + if ((unsigned)len != strlen(getname)) FAIL("fdt_get_name(%s) returned length %d instead of %zd", path, len, strlen(getname)); diff --git a/tests/integer-expressions.c b/tests/integer-expressions.c index 6f33d81..2f164d9 100644 --- a/tests/integer-expressions.c +++ b/tests/integer-expressions.c @@ -59,7 +59,7 @@ int main(int argc, char *argv[]) void *fdt; const fdt32_t *res; int reslen; - int i; + unsigned int i; test_init(argc, argv); diff --git a/tests/nopulate.c b/tests/nopulate.c index 2ae1753..e06a0b3 100644 --- a/tests/nopulate.c +++ b/tests/nopulate.c @@ -43,7 +43,8 @@ static int nopulate_struct(char *buf, const char *fdt) int main(int argc, char *argv[]) { char *fdt, *fdt2, *buf; - int newsize, struct_start, struct_end_old, struct_end_new, delta; + int newsize, struct_end_old, struct_end_new, delta; + unsigned int struct_start; const char *inname; char outname[PATH_MAX]; diff --git a/tests/overlay.c b/tests/overlay.c index 91afa72..f3dd310 100644 --- a/tests/overlay.c +++ b/tests/overlay.c @@ -35,7 +35,11 @@ static int fdt_getprop_u32_by_poffset(void *fdt, const char *path, return node_off; val = fdt_getprop(fdt, node_off, name, &len); - if (!val || (len < (sizeof(uint32_t) * (poffset + 1)))) + if (val && len < 0) + FAIL("fdt_getprop() returns negative length"); + if (!val && len < 0) + return len; + if (!val || ((unsigned)len < (sizeof(uint32_t) * (poffset + 1)))) return -FDT_ERR_NOTFOUND; *out = fdt32_to_cpu(*(val + poffset)); diff --git a/tests/phandle_format.c b/tests/phandle_format.c index d00618f..0febb32 100644 --- a/tests/phandle_format.c +++ b/tests/phandle_format.c @@ -45,7 +45,7 @@ int main(int argc, char *argv[]) FAIL("fdt_path_offset(/node4): %s", fdt_strerror(n4)); h4 = fdt_get_phandle(fdt, n4); - if ((h4 == 0) || (h4 == -1)) + if ((h4 == 0) || (h4 == ~0U)) FAIL("/node4 has bad phandle 0x%x\n", h4); if (phandle_format & PHANDLE_LEGACY) diff --git a/tests/property_iterate.c b/tests/property_iterate.c index 9a67f49..0b6af9b 100644 --- a/tests/property_iterate.c +++ b/tests/property_iterate.c @@ -23,7 +23,7 @@ static void test_node(void *fdt, int parent_offset) uint32_t properties; const fdt32_t *prop; int offset, property; - int count; + unsigned int count; int len; /* diff --git a/tests/references.c b/tests/references.c index d18e722..cb1daaa 100644 --- a/tests/references.c +++ b/tests/references.c @@ -106,7 +106,7 @@ int main(int argc, char *argv[]) if ((h4 == 0x2000) || (h4 == 0x1) || (h4 == 0)) FAIL("/node4 has bad phandle, 0x%x", h4); - if ((h5 == 0) || (h5 == -1)) + if ((h5 == 0) || (h5 == ~0U)) FAIL("/node5 has bad phandle, 0x%x", h5); if ((h5 == h4) || (h5 == h2) || (h5 == h1)) FAIL("/node5 has duplicate phandle, 0x%x", h5); diff --git a/tests/set_name.c b/tests/set_name.c index a62cb58..5020305 100644 --- a/tests/set_name.c +++ b/tests/set_name.c @@ -39,7 +39,11 @@ static void check_set_name(void *fdt, const char *path, const char *newname) FAIL("fdt_get_name(%s) returned \"%s\" instead of \"%s\"", path, getname, oldname); - if (len != strlen(getname)) + if (len < 0) + FAIL("fdt_get_name(%s) returned negative length: %d", + path, len); + + if ((unsigned)len != strlen(getname)) FAIL("fdt_get_name(%s) returned length %d instead of %zd", path, len, strlen(getname)); @@ -51,12 +55,14 @@ static void check_set_name(void *fdt, const char *path, const char *newname) getname = fdt_get_name(fdt, offset, &len); if (!getname) FAIL("fdt_get_name(%d): %s", offset, fdt_strerror(len)); + if (len < 0) + FAIL("negative name length (%d) for returned node name\n", len); if (strcmp(getname, newname) != 0) FAIL("fdt_get_name(%s) returned \"%s\" instead of \"%s\"", path, getname, newname); - if (len != strlen(getname)) + if ((unsigned)len != strlen(getname)) FAIL("fdt_get_name(%s) returned length %d instead of %zd", path, len, strlen(getname)); } diff --git a/tests/subnode_iterate.c b/tests/subnode_iterate.c index 2dc9b2d..2553a51 100644 --- a/tests/subnode_iterate.c +++ b/tests/subnode_iterate.c @@ -23,7 +23,7 @@ static void test_node(void *fdt, int parent_offset) uint32_t subnodes; const fdt32_t *prop; int offset; - int count; + unsigned int count; int len; /* This property indicates the number of subnodes to expect */ diff --git a/tests/tests.h b/tests/tests.h index 1017366..bf8f23c 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -83,7 +83,7 @@ void cleanup(void); void check_mem_rsv(void *fdt, int n, uint64_t addr, uint64_t size); void check_property(void *fdt, int nodeoffset, const char *name, - int len, const void *val); + unsigned int len, const void *val); #define check_property_cell(fdt, nodeoffset, name, val) \ ({ \ fdt32_t x = cpu_to_fdt32(val); \ diff --git a/tests/testutils.c b/tests/testutils.c index 5e494c5..10129c0 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -88,7 +88,7 @@ void check_mem_rsv(void *fdt, int n, uint64_t addr, uint64_t size) } void check_property(void *fdt, int nodeoffset, const char *name, - int len, const void *val) + unsigned int len, const void *val) { const struct fdt_property *prop; int retlen, namelen; @@ -101,6 +101,9 @@ void check_property(void *fdt, int nodeoffset, const char *name, if (! prop) FAIL("Error retrieving \"%s\" pointer: %s", name, fdt_strerror(retlen)); + if (retlen < 0) + FAIL("negative name length (%d) for returned property\n", + retlen); tag = fdt32_to_cpu(prop->tag); nameoff = fdt32_to_cpu(prop->nameoff); @@ -112,13 +115,16 @@ void check_property(void *fdt, int nodeoffset, const char *name, propname = fdt_get_string(fdt, nameoff, &namelen); if (!propname) FAIL("Couldn't get property name: %s", fdt_strerror(namelen)); - if (namelen != strlen(propname)) + if (namelen < 0) + FAIL("negative name length (%d) for returned string\n", + namelen); + if ((unsigned)namelen != strlen(propname)) FAIL("Incorrect prop name length: %d instead of %zd", namelen, strlen(propname)); if (!streq(propname, name)) FAIL("Property name mismatch \"%s\" instead of \"%s\"", propname, name); - if (proplen != retlen) + if (proplen != (unsigned)retlen) FAIL("Length retrieved for \"%s\" by fdt_get_property()" " differs from stored length (%d != %d)", name, retlen, proplen); From 910221185560fe0c5dc0997dd7d3b472a0a7cdea Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 18 Jun 2021 18:20:27 +0100 Subject: [PATCH 065/104] fdtget: Fix signedness comparisons warnings With -Wsign-compare, compilers warn about a mismatching signedness in the different legs of the conditional operator, in fdtget.c. In the questionable expression, we are constructing a 16-bit value out of two unsigned 8-bit values, however are relying on the compiler's automatic expansion of the uint8_t to a larger type, to survive the left shift. This larger type happens to be an "int", so this part of the expression becomes signed. Fix this by explicitly blowing up the uint8_t to a larger *unsigned* type, before doing the left shift. And while we are at it, convert the hardly readable conditional operator usage into a sane switch/case expression. This fixes "make fdtget", when compiled with -Wsign-compare. Signed-off-by: Andre Przywara Message-Id: <20210618172030.9684-3-andre.przywara@arm.com> Signed-off-by: David Gibson --- fdtget.c | 10 ++++++++-- libfdt/libfdt.h | 7 +++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/fdtget.c b/fdtget.c index 777582e..54fc6a0 100644 --- a/fdtget.c +++ b/fdtget.c @@ -62,8 +62,14 @@ static int show_cell_list(struct display_info *disp, const char *data, int len, for (i = 0; i < len; i += size, p += size) { if (i) printf(" "); - value = size == 4 ? fdt32_ld((const fdt32_t *)p) : - size == 2 ? (*p << 8) | p[1] : *p; + switch (size) { + case 4: value = fdt32_ld((const fdt32_t *)p); break; + case 2: value = fdt16_ld((const fdt16_t *)p); break; + case 1: + default: + value = *p; + break; + } printf(fmt, value); } diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h index 73467f7..7f117e8 100644 --- a/libfdt/libfdt.h +++ b/libfdt/libfdt.h @@ -131,6 +131,13 @@ uint32_t fdt_next_tag(const void *fdt, int offset, int *nextoffset); * to work even with unaligned pointers on platforms (such as ARMv5) that don't * like unaligned loads and stores. */ +static inline uint16_t fdt16_ld(const fdt16_t *p) +{ + const uint8_t *bp = (const uint8_t *)p; + + return ((uint16_t)bp[0] << 8) | bp[1]; +} + static inline uint32_t fdt32_ld(const fdt32_t *p) { const uint8_t *bp = (const uint8_t *)p; From 69bed6c2418f484561263aadbb886ffb925e6b38 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 18 Jun 2021 18:20:28 +0100 Subject: [PATCH 066/104] dtc: Wrap phandle validity check In several places we check for a returned phandle value to be valid, for that it must not be 0 or "-1". Wrap this check in a static inline function in dtc.h, and use ~0U instead of -1 on the way, to keep everything in the unsigned realm. Signed-off-by: Andre Przywara Message-Id: <20210618172030.9684-4-andre.przywara@arm.com> Signed-off-by: David Gibson --- checks.c | 10 +++++----- dtc.h | 5 +++++ livetree.c | 4 ++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/checks.c b/checks.c index e6c7c3e..cb71c73 100644 --- a/checks.c +++ b/checks.c @@ -520,7 +520,7 @@ static cell_t check_phandle_prop(struct check *c, struct dt_info *dti, phandle = propval_cell(prop); - if ((phandle == 0) || (phandle == -1)) { + if (!phandle_is_valid(phandle)) { FAIL_PROP(c, dti, node, prop, "bad value (0x%x) in %s property", phandle, prop->name); return 0; @@ -1400,14 +1400,14 @@ static void check_property_phandle_args(struct check *c, for (cell = 0; cell < prop->val.len / sizeof(cell_t); cell += cellsize + 1) { struct node *provider_node; struct property *cellprop; - int phandle; + cell_t phandle; phandle = propval_cell_n(prop, cell); /* * Some bindings use a cell value 0 or -1 to skip over optional * entries when each index position has a specific definition. */ - if (phandle == 0 || phandle == -1) { + if (!phandle_is_valid(phandle)) { /* Give up if this is an overlay with external references */ if (dti->dtsflags & DTSF_PLUGIN) break; @@ -1615,7 +1615,7 @@ static void check_interrupts_property(struct check *c, prop = get_property(parent, "interrupt-parent"); if (prop) { phandle = propval_cell(prop); - if ((phandle == 0) || (phandle == -1)) { + if (!phandle_is_valid(phandle)) { /* Give up if this is an overlay with * external references */ if (dti->dtsflags & DTSF_PLUGIN) @@ -1772,7 +1772,7 @@ static struct node *get_remote_endpoint(struct check *c, struct dt_info *dti, phandle = propval_cell(prop); /* Give up if this is an overlay with external references */ - if (phandle == 0 || phandle == -1) + if (!phandle_is_valid(phandle)) return NULL; node = get_node_by_phandle(dti->dt, phandle); diff --git a/dtc.h b/dtc.h index 47664f2..cf2c6ac 100644 --- a/dtc.h +++ b/dtc.h @@ -51,6 +51,11 @@ extern int annotate; /* annotate .dts with input source location */ typedef uint32_t cell_t; +static inline bool phandle_is_valid(cell_t phandle) +{ + return phandle != 0 && phandle != ~0U; +} + static inline uint16_t dtb_ld16(const void *p) { const uint8_t *bp = (const uint8_t *)p; diff --git a/livetree.c b/livetree.c index dfbae65..cc61237 100644 --- a/livetree.c +++ b/livetree.c @@ -559,7 +559,7 @@ struct node *get_node_by_phandle(struct node *tree, cell_t phandle) { struct node *child, *node; - if ((phandle == 0) || (phandle == -1)) { + if (!phandle_is_valid(phandle)) { assert(generate_fixups); return NULL; } @@ -594,7 +594,7 @@ cell_t get_node_phandle(struct node *root, struct node *node) static cell_t phandle = 1; /* FIXME: ick, static local */ struct data d = empty_data; - if ((node->phandle != 0) && (node->phandle != -1)) + if (phandle_is_valid(node->phandle)) return node->phandle; while (get_node_by_phandle(root, phandle)) From b587787ef38802e43972e1b7a36a8358ae25db86 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 18 Jun 2021 18:20:29 +0100 Subject: [PATCH 067/104] checks: Fix signedness comparisons warnings With -Wsign-compare, compilers warn about a mismatching signedness in comparisons in various parts in checks.c. Fix those by making all affected variables unsigned. This covers return values of the (unsigned) size_t type, phandles, variables holding sizes in general and loop counters only ever counting positives values. Signed-off-by: Andre Przywara Message-Id: <20210618172030.9684-5-andre.przywara@arm.com> Signed-off-by: David Gibson --- checks.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/checks.c b/checks.c index cb71c73..e2690e9 100644 --- a/checks.c +++ b/checks.c @@ -312,7 +312,7 @@ ERROR(duplicate_property_names, check_duplicate_property_names, NULL); static void check_node_name_chars(struct check *c, struct dt_info *dti, struct node *node) { - int n = strspn(node->name, c->data); + size_t n = strspn(node->name, c->data); if (n < strlen(node->name)) FAIL(c, dti, node, "Bad character '%c' in node name", @@ -386,7 +386,7 @@ static void check_property_name_chars(struct check *c, struct dt_info *dti, struct property *prop; for_each_property(node, prop) { - int n = strspn(prop->name, c->data); + size_t n = strspn(prop->name, c->data); if (n < strlen(prop->name)) FAIL_PROP(c, dti, node, prop, "Bad character '%c' in property name", @@ -403,7 +403,7 @@ static void check_property_name_chars_strict(struct check *c, for_each_property(node, prop) { const char *name = prop->name; - int n = strspn(name, c->data); + size_t n = strspn(name, c->data); if (n == strlen(prop->name)) continue; @@ -579,7 +579,7 @@ static void check_name_properties(struct check *c, struct dt_info *dti, if (!prop) return; /* No name property, that's fine */ - if ((prop->val.len != node->basenamelen+1) + if ((prop->val.len != node->basenamelen + 1U) || (memcmp(prop->val.val, node->name, node->basenamelen) != 0)) { FAIL(c, dti, node, "\"name\" property is incorrect (\"%s\" instead" " of base node name)", prop->val.val); @@ -1388,7 +1388,7 @@ static void check_property_phandle_args(struct check *c, const struct provider *provider) { struct node *root = dti->dt; - int cell, cellsize = 0; + unsigned int cell, cellsize = 0; if (!is_multiple_of(prop->val.len, sizeof(cell_t))) { FAIL_PROP(c, dti, node, prop, @@ -1596,7 +1596,7 @@ static void check_interrupts_property(struct check *c, struct node *root = dti->dt; struct node *irq_node = NULL, *parent = node; struct property *irq_prop, *prop = NULL; - int irq_cells, phandle; + cell_t irq_cells, phandle; irq_prop = get_property(node, "interrupts"); if (!irq_prop) @@ -1762,7 +1762,7 @@ WARNING(graph_port, check_graph_port, NULL, &graph_nodes); static struct node *get_remote_endpoint(struct check *c, struct dt_info *dti, struct node *endpoint) { - int phandle; + cell_t phandle; struct node *node; struct property *prop; @@ -1910,7 +1910,7 @@ static void enable_warning_error(struct check *c, bool warn, bool error) static void disable_warning_error(struct check *c, bool warn, bool error) { - int i; + unsigned int i; /* Lowering level, also lower it for things this is the prereq * for */ @@ -1931,7 +1931,7 @@ static void disable_warning_error(struct check *c, bool warn, bool error) void parse_checks_option(bool warn, bool error, const char *arg) { - int i; + unsigned int i; const char *name = arg; bool enable = true; @@ -1958,7 +1958,7 @@ void parse_checks_option(bool warn, bool error, const char *arg) void process_checks(bool force, struct dt_info *dti) { - int i; + unsigned int i; int error = 0; for (i = 0; i < ARRAY_SIZE(check_table); i++) { From 72d09e2682a41589b46b8cdd4c393b8ed47ac266 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Fri, 18 Jun 2021 18:20:30 +0100 Subject: [PATCH 068/104] Makefile: add -Wsign-compare to warning options Now that all signedness comparison warnings in the source tree have been fixed, let's enable the warning option, to avoid them creeping in again. Signed-off-by: Andre Przywara Message-Id: <20210618172030.9684-6-andre.przywara@arm.com> Signed-off-by: David Gibson --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ea8c659..ee77115 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ CONFIG_LOCALVERSION = ASSUME_MASK ?= 0 CPPFLAGS = -I libfdt -I . -DFDT_ASSUME_MASK=$(ASSUME_MASK) -WARNINGS = -Wall -Wpointer-arith -Wcast-qual -Wnested-externs \ +WARNINGS = -Wall -Wpointer-arith -Wcast-qual -Wnested-externs -Wsign-compare \ -Wstrict-prototypes -Wmissing-prototypes -Wredundant-decls -Wshadow CFLAGS = -g -Os $(SHAREDLIB_CFLAGS) -Werror $(WARNINGS) $(EXTRA_CFLAGS) From 69595a167f06c4482ce784e30df1ac9b16ceb5b0 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Tue, 29 Jun 2021 13:43:04 +0200 Subject: [PATCH 069/104] checks: Fix bus-range check The upper limit of the bus-range is specified by the second cell of the bus-range property. Signed-off-by: Thierry Reding Message-Id: <20210629114304.2451114-1-thierry.reding@gmail.com> Signed-off-by: David Gibson --- checks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checks.c b/checks.c index e2690e9..fb3fc9c 100644 --- a/checks.c +++ b/checks.c @@ -892,7 +892,7 @@ static void check_pci_device_bus_num(struct check *c, struct dt_info *dti, struc } else { cells = (cell_t *)prop->val.val; min_bus = fdt32_to_cpu(cells[0]); - max_bus = fdt32_to_cpu(cells[0]); + max_bus = fdt32_to_cpu(cells[1]); } if ((bus_num < min_bus) || (bus_num > max_bus)) FAIL_PROP(c, dti, node, prop, "PCI bus number %d out of range, expected (%d - %d)", From 0869f8269161843a46c0cac0e35b4dc9324bf1a6 Mon Sep 17 00:00:00 2001 From: Georg Kotheimer Date: Sat, 31 Jul 2021 13:48:42 +0200 Subject: [PATCH 070/104] libfdt: Add ALIGNMENT error string The ALIGNMENT error was missing a string, leading to being returned. Signed-off-by: Georg Kotheimer Signed-off-by: David Gibson --- libfdt/fdt_strerror.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libfdt/fdt_strerror.c b/libfdt/fdt_strerror.c index b435693..d852b77 100644 --- a/libfdt/fdt_strerror.c +++ b/libfdt/fdt_strerror.c @@ -39,6 +39,7 @@ static struct fdt_errtabent fdt_errtable[] = { FDT_ERRTABENT(FDT_ERR_BADOVERLAY), FDT_ERRTABENT(FDT_ERR_NOPHANDLES), FDT_ERRTABENT(FDT_ERR_BADFLAGS), + FDT_ERRTABENT(FDT_ERR_ALIGNMENT), }; #define FDT_ERRTABSIZE ((int)(sizeof(fdt_errtable) / sizeof(fdt_errtable[0]))) From 5eb5927d81ee6036f45c4e1bd89ae66ed325d721 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 25 Aug 2021 16:13:50 +0400 Subject: [PATCH 071/104] fdtdump: fix -Werror=int-to-pointer-cast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With mingw64-gcc, the compiler complains with various warnings: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] Signed-off-by: Marc-André Lureau Message-Id: <20210825121350.213551-1-marcandre.lureau@redhat.com> Acked-by: Rob Herring Signed-off-by: David Gibson --- fdtdump.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fdtdump.c b/fdtdump.c index bdc0f94..d424869 100644 --- a/fdtdump.c +++ b/fdtdump.c @@ -21,7 +21,7 @@ #define MAX_VERSION 17U #define ALIGN(x, a) (((x) + ((a) - 1)) & ~((a) - 1)) -#define PALIGN(p, a) ((void *)(ALIGN((unsigned long)(p), (a)))) +#define PALIGN(p, a) ((void *)(ALIGN((uintptr_t)(p), (a)))) #define GET_CELL(p) (p += 4, *((const fdt32_t *)(p-4))) static const char *tagname(uint32_t tag) From ff3a30c115ad7354689dc7858604356ecb7f9b1c Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Tue, 27 Jul 2021 12:30:19 -0600 Subject: [PATCH 072/104] asm: Use .asciz and .ascii instead of .string We use the .string pseudo-op both in some of our test assembly files and in our -Oasm output. We expect this to emit a \0 terminated string into the .o file. However for certain targets (e.g. HP PA-RISC) it doesn't include the \0. Use .asciz instead, which explicitly does what we want. There's also one place we can use .ascii (which explicitly emits a string *without* \0 termination) instead of multiple .byte directives. Signed-off-by: David Gibson --- dtc.h | 23 ++++++++++++++++++++- flattree.c | 6 +++--- tests/base01.asm | 38 +++++++++++++++++------------------ tests/test01.asm | 52 ++++++++++++++++++++++++------------------------ tests/trees.S | 10 ++++------ treesource.c | 23 +-------------------- 6 files changed, 75 insertions(+), 77 deletions(-) diff --git a/dtc.h b/dtc.h index cf2c6ac..0a1f549 100644 --- a/dtc.h +++ b/dtc.h @@ -116,6 +116,12 @@ enum markertype { TYPE_UINT64, TYPE_STRING, }; + +static inline bool is_type_marker(enum markertype type) +{ + return type >= TYPE_UINT8; +} + extern const char *markername(enum markertype markertype); struct marker { @@ -140,7 +146,22 @@ struct data { for_each_marker(m) \ if ((m)->type == (t)) -size_t type_marker_length(struct marker *m); +static inline struct marker *next_type_marker(struct marker *m) +{ + for_each_marker(m) + if (is_type_marker(m->type)) + break; + return m; +} + +static inline size_t type_marker_length(struct marker *m) +{ + struct marker *next = next_type_marker(m->next); + + if (next) + return next->offset - m->offset; + return 0; +} void data_free(struct data d); diff --git a/flattree.c b/flattree.c index 3d0204f..5254aeb 100644 --- a/flattree.c +++ b/flattree.c @@ -134,9 +134,9 @@ static void asm_emit_string(void *e, const char *str, int len) FILE *f = e; if (len != 0) - fprintf(f, "\t.string\t\"%.*s\"\n", len, str); + fprintf(f, "\t.asciz\t\"%.*s\"\n", len, str); else - fprintf(f, "\t.string\t\"%s\"\n", str); + fprintf(f, "\t.asciz\t\"%s\"\n", str); } static void asm_emit_align(void *e, int a) @@ -438,7 +438,7 @@ static void dump_stringtable_asm(FILE *f, struct data strbuf) while (p < (strbuf.val + strbuf.len)) { len = strlen(p); - fprintf(f, "\t.string \"%s\"\n", p); + fprintf(f, "\t.asciz \"%s\"\n", p); p += len+1; } } diff --git a/tests/base01.asm b/tests/base01.asm index 266e446..63f2eca 100644 --- a/tests/base01.asm +++ b/tests/base01.asm @@ -32,7 +32,7 @@ _dt_reserve_map: dt_struct_start: _dt_struct_start: .long OF_DT_BEGIN_NODE - .string "" + .asciz "" .balign 4 .long OF_DT_PROP .long 0xa @@ -58,7 +58,7 @@ _dt_struct_start: .long 0x2 .balign 4 .long OF_DT_BEGIN_NODE - .string "memory@0" + .asciz "memory@0" .balign 4 .long OF_DT_PROP .long 0x7 @@ -77,7 +77,7 @@ _dt_struct_start: .balign 4 .long OF_DT_END_NODE .long OF_DT_BEGIN_NODE - .string "cpus" + .asciz "cpus" .balign 4 .long OF_DT_PROP .long 0x4 @@ -151,22 +151,22 @@ _dt_struct_end: .globl dt_strings_start dt_strings_start: _dt_strings_start: - .string "model" - .string "compatible" - .string "#address-cells" - .string "#size-cells" - .string "device_type" - .string "reg" - .string "d10" - .string "d23" - .string "b101" - .string "o17" - .string "hd00d" - .string "stuff" - .string "bad-d-1" - .string "bad-d-2" - .string "bad-o-1" - .string "bad-o-2" + .asciz "model" + .asciz "compatible" + .asciz "#address-cells" + .asciz "#size-cells" + .asciz "device_type" + .asciz "reg" + .asciz "d10" + .asciz "d23" + .asciz "b101" + .asciz "o17" + .asciz "hd00d" + .asciz "stuff" + .asciz "bad-d-1" + .asciz "bad-d-2" + .asciz "bad-o-1" + .asciz "bad-o-2" .globl dt_strings_end dt_strings_end: _dt_strings_end: diff --git a/tests/test01.asm b/tests/test01.asm index bbf66c7..73fa603 100644 --- a/tests/test01.asm +++ b/tests/test01.asm @@ -44,7 +44,7 @@ _dt_reserve_map: dt_struct_start: _dt_struct_start: .long OF_DT_BEGIN_NODE - .string "" + .asciz "" .balign 4 .long OF_DT_PROP .long 0xc @@ -76,7 +76,7 @@ _dt_struct_start: .long 0x2 .balign 4 .long OF_DT_BEGIN_NODE - .string "cpus" + .asciz "cpus" .balign 4 .long OF_DT_PROP .long 0x4 @@ -94,7 +94,7 @@ _dt_struct_start: .long 0x0 .balign 4 .long OF_DT_BEGIN_NODE - .string "PowerPC,970@0" + .asciz "PowerPC,970@0" .balign 4 .long OF_DT_PROP .long 0xc @@ -139,7 +139,7 @@ _dt_struct_start: .balign 4 .long OF_DT_END_NODE .long OF_DT_BEGIN_NODE - .string "PowerPC,970@1" + .asciz "PowerPC,970@1" .balign 4 .long OF_DT_PROP .long 0xc @@ -181,7 +181,7 @@ _dt_struct_start: .long OF_DT_END_NODE .long OF_DT_END_NODE .long OF_DT_BEGIN_NODE - .string "randomnode" + .asciz "randomnode" .balign 4 .long OF_DT_PROP .long 0x13 @@ -216,7 +216,7 @@ _dt_struct_start: .balign 4 .long OF_DT_END_NODE .long OF_DT_BEGIN_NODE - .string "memory@0" + .asciz "memory@0" .balign 4 .long OF_DT_PROP .long 0x7 @@ -242,7 +242,7 @@ memreg: .balign 4 .long OF_DT_END_NODE .long OF_DT_BEGIN_NODE - .string "chosen" + .asciz "chosen" .balign 4 .long OF_DT_PROP .long 0xf @@ -267,25 +267,25 @@ _dt_struct_end: .globl dt_strings_start dt_strings_start: _dt_strings_start: - .string "model" - .string "compatible" - .string "#address-cells" - .string "#size-cells" - .string "linux,phandle" - .string "name" - .string "device_type" - .string "reg" - .string "clock-frequency" - .string "timebase-frequency" - .string "linux,boot-cpu" - .string "i-cache-size" - .string "d-cache-size" - .string "string" - .string "blob" - .string "ref" - .string "mixed" - .string "bootargs" - .string "linux,platform" + .asciz "model" + .asciz "compatible" + .asciz "#address-cells" + .asciz "#size-cells" + .asciz "linux,phandle" + .asciz "name" + .asciz "device_type" + .asciz "reg" + .asciz "clock-frequency" + .asciz "timebase-frequency" + .asciz "linux,boot-cpu" + .asciz "i-cache-size" + .asciz "d-cache-size" + .asciz "string" + .asciz "blob" + .asciz "ref" + .asciz "mixed" + .asciz "bootargs" + .asciz "linux,platform" .globl dt_strings_end dt_strings_end: _dt_strings_end: diff --git a/tests/trees.S b/tests/trees.S index 95d599d..ab0871f 100644 --- a/tests/trees.S +++ b/tests/trees.S @@ -54,13 +54,13 @@ tree##_rsvmap_end: ; #define PROP_STR(tree, name, str) \ PROPHDR(tree, name, 55f - 54f) \ 54: \ - .string str ; \ + .asciz str ; \ 55: \ .balign 4 ; #define BEGIN_NODE(name) \ FDTLONG(FDT_BEGIN_NODE) ; \ - .string name ; \ + .asciz name ; \ .balign 4 ; #define END_NODE \ @@ -68,7 +68,7 @@ tree##_rsvmap_end: ; #define STRING(tree, name, str) \ tree##_##name: ; \ - .string str ; + .asciz str ; .data @@ -253,9 +253,7 @@ truncated_string_struct_end: truncated_string_strings: STRING(truncated_string, good_string, "good") truncated_string_bad_string: - .byte 'b' - .byte 'a' - .byte 'd' + .ascii "bad" /* NOTE: terminating \0 deliberately missing */ truncated_string_strings_end: truncated_string_end: diff --git a/treesource.c b/treesource.c index 061ba8c..db2ff69 100644 --- a/treesource.c +++ b/treesource.c @@ -124,27 +124,6 @@ static void write_propval_int(FILE *f, const char *p, size_t len, size_t width) } } -static bool has_data_type_information(struct marker *m) -{ - return m->type >= TYPE_UINT8; -} - -static struct marker *next_type_marker(struct marker *m) -{ - while (m && !has_data_type_information(m)) - m = m->next; - return m; -} - -size_t type_marker_length(struct marker *m) -{ - struct marker *next = next_type_marker(m->next); - - if (next) - return next->offset - m->offset; - return 0; -} - static const char *delim_start[] = { [TYPE_UINT8] = "[", [TYPE_UINT16] = "/bits/ 16 <", @@ -230,7 +209,7 @@ static void write_propval(FILE *f, struct property *prop) size_t data_len = type_marker_length(m) ? : len - m->offset; const char *p = &prop->val.val[m->offset]; - if (has_data_type_information(m)) { + if (is_type_marker(m->type)) { emit_type = m->type; fprintf(f, " %s", delim_start[emit_type]); } else if (m->type == LABEL) From d24cc189dca6148eedf9dc9e2d45144b3851dae0 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 24 Sep 2021 21:14:34 +1000 Subject: [PATCH 073/104] asm: Use assembler macros instead of cpp macros tests/trees.S is a weird thing: a portable aseembler file, used to produce a specific binary output. Currently it uses CPP macros quite heavily to construct the dtbs we want (including some partial and broken trees). Using cpp has the side effect that we need to use ; separators between instructions (or, rather, pseudo-ops), because cpp won't expand newlines. However, it turns out that while ; is a suitable separator on most targets, it doesn't work for all of them (e.g. HP PA-RISC). Switch to using the assembler's inbuilt macros rather than CPP, so that we can use genuine newlines. Signed-off-by: David Gibson --- tests/trees.S | 361 ++++++++++++++++++++++++++------------------------ 1 file changed, 187 insertions(+), 174 deletions(-) diff --git a/tests/trees.S b/tests/trees.S index ab0871f..d69f7f1 100644 --- a/tests/trees.S +++ b/tests/trees.S @@ -1,174 +1,187 @@ #include #include "testdata.h" -#define FDTLONG(val) \ - .byte ((val) >> 24) & 0xff ; \ - .byte ((val) >> 16) & 0xff ; \ - .byte ((val) >> 8) & 0xff ; \ - .byte (val) & 0xff ; + .macro fdtlong val + .byte ((\val) >> 24) & 0xff + .byte ((\val) >> 16) & 0xff + .byte ((\val) >> 8) & 0xff + .byte (\val) & 0xff + .endm -#define TREE_HDR(tree) \ - .balign 8 ; \ - .globl tree ; \ -tree: \ - FDTLONG(FDT_MAGIC) ; \ - FDTLONG(tree##_end - tree) ; \ - FDTLONG(tree##_struct - tree) ; \ - FDTLONG(tree##_strings - tree) ; \ - FDTLONG(tree##_rsvmap - tree) ; \ - FDTLONG(0x11) ; \ - FDTLONG(0x10) ; \ - FDTLONG(0) ; \ - FDTLONG(tree##_strings_end - tree##_strings) ; \ - FDTLONG(tree##_struct_end - tree##_struct) ; + .macro treehdr tree + .balign 8 + .globl \tree +\tree : + fdtlong FDT_MAGIC + fdtlong (\tree\()_end - \tree) + fdtlong (\tree\()_struct - \tree) + fdtlong (\tree\()_strings - \tree) + fdtlong (\tree\()_rsvmap - \tree) + fdtlong 0x11 + fdtlong 0x10 + fdtlong 0 + fdtlong (\tree\()_strings_end - \tree\()_strings) + fdtlong (\tree\()_struct_end - \tree\()_struct) + .endm -#define RSVMAP_ENTRY(addrh, addrl, lenh, lenl) \ - FDTLONG(addrh) ; \ - FDTLONG(addrl) ; \ - FDTLONG(lenh) ; \ - FDTLONG(lenl) + .macro rsvmape addrh, addrl, lenh, lenl + fdtlong \addrh + fdtlong \addrl + fdtlong \lenh + fdtlong \lenl + .endm -#define EMPTY_RSVMAP(tree) \ - .balign 8 ; \ -tree##_rsvmap: ; \ - RSVMAP_ENTRY(0, 0, 0, 0) \ -tree##_rsvmap_end: ; + .macro empty_rsvmap tree + .balign 8 +\tree\()_rsvmap: + rsvmape 0, 0, 0, 0 +\tree\()_rsvmap_end: + .endm -#define PROPHDR(tree, name, len) \ - FDTLONG(FDT_PROP) ; \ - FDTLONG(len) ; \ - FDTLONG(tree##_##name - tree##_strings) ; + .macro prophdr tree, name, len + fdtlong FDT_PROP + fdtlong \len + fdtlong (\tree\()_\name - \tree\()_strings) + .endm -#define PROP_EMPTY(tree, name) \ - PROPHDR(tree, name, 0) ; + .macro propnil tree, name + prophdr \tree, \name, 0 + .endm -#define PROP_INT(tree, name, val) \ - PROPHDR(tree, name, 4) \ - FDTLONG(val) ; + .macro propu32 tree, name, val + prophdr \tree, \name, 4 + fdtlong \val + .endm -#define PROP_INT64(tree, name, valh, vall) \ - PROPHDR(tree, name, 8) \ - FDTLONG(valh) ; \ - FDTLONG(vall) ; + .macro propu64 tree, name, valh, vall + prophdr \tree, \name, 8 + fdtlong \valh + fdtlong \vall + .endm -#define PROP_STR(tree, name, str) \ - PROPHDR(tree, name, 55f - 54f) \ -54: \ - .asciz str ; \ -55: \ - .balign 4 ; + .macro propstr tree, name, str:vararg + prophdr \tree, \name, (55f - 54f) +54: + .asciz \str +55: + .balign 4 + .endm -#define BEGIN_NODE(name) \ - FDTLONG(FDT_BEGIN_NODE) ; \ - .asciz name ; \ - .balign 4 ; + .macro beginn name:vararg + fdtlong FDT_BEGIN_NODE + .asciz \name + .balign 4 + .endm -#define END_NODE \ - FDTLONG(FDT_END_NODE) ; + .macro endn + fdtlong FDT_END_NODE + .endm + + .macro string tree, name, str:vararg +\tree\()_\name : + .asciz \str + .endm -#define STRING(tree, name, str) \ -tree##_##name: ; \ - .asciz str ; .data - TREE_HDR(test_tree1) + treehdr test_tree1 .balign 8 test_tree1_rsvmap: - RSVMAP_ENTRY(TEST_ADDR_1H, TEST_ADDR_1L, TEST_SIZE_1H, TEST_SIZE_1L) - RSVMAP_ENTRY(TEST_ADDR_2H, TEST_ADDR_2L, TEST_SIZE_2H, TEST_SIZE_2L) - RSVMAP_ENTRY(0, 0, 0, 0) + rsvmape TEST_ADDR_1H, TEST_ADDR_1L, TEST_SIZE_1H, TEST_SIZE_1L + rsvmape TEST_ADDR_2H, TEST_ADDR_2L, TEST_SIZE_2H, TEST_SIZE_2L + rsvmape 0, 0, 0, 0 test_tree1_rsvmap_end: test_tree1_struct: - BEGIN_NODE("") - PROP_STR(test_tree1, compatible, "test_tree1") - PROP_INT(test_tree1, prop_int, TEST_VALUE_1) - PROP_INT64(test_tree1, prop_int64, TEST_VALUE64_1H, TEST_VALUE64_1L) - PROP_STR(test_tree1, prop_str, TEST_STRING_1) - PROP_INT(test_tree1, address_cells, 1) - PROP_INT(test_tree1, size_cells, 0) + beginn "" + propstr test_tree1, compatible, "test_tree1" + propu32 test_tree1, prop_int, TEST_VALUE_1 + propu64 test_tree1, prop_int64, TEST_VALUE64_1H, TEST_VALUE64_1L + propstr test_tree1, prop_str, TEST_STRING_1 + propu32 test_tree1, address_cells, 1 + propu32 test_tree1, size_cells, 0 - BEGIN_NODE("subnode@1") - PROP_STR(test_tree1, compatible, "subnode1") - PROP_INT(test_tree1, reg, 1) - PROP_INT(test_tree1, prop_int, TEST_VALUE_1) + beginn "subnode@1" + propstr test_tree1, compatible, "subnode1" + propu32 test_tree1, reg, 1 + propu32 test_tree1, prop_int, TEST_VALUE_1 - BEGIN_NODE("subsubnode") - PROP_STR(test_tree1, compatible, "subsubnode1\0subsubnode") - PROP_STR(test_tree1, placeholder, "this is a placeholder string\0string2") - PROP_INT(test_tree1, prop_int, TEST_VALUE_1) - END_NODE + beginn "subsubnode" + propstr test_tree1, compatible, "subsubnode1\0subsubnode" + propstr test_tree1, placeholder, "this is a placeholder string\0string2" + propu32 test_tree1, prop_int, TEST_VALUE_1 + endn - BEGIN_NODE("ss1") - END_NODE + beginn "ss1" + endn - END_NODE + endn - BEGIN_NODE("subnode@2") - PROP_INT(test_tree1, reg, 2) - PROP_INT(test_tree1, linux_phandle, PHANDLE_1) - PROP_INT(test_tree1, prop_int, TEST_VALUE_2) - PROP_INT(test_tree1, address_cells, 1) - PROP_INT(test_tree1, size_cells, 0) + beginn "subnode@2" + propu32 test_tree1, reg, 2 + propu32 test_tree1, linux_phandle, PHANDLE_1 + propu32 test_tree1, prop_int, TEST_VALUE_2 + propu32 test_tree1, address_cells, 1 + propu32 test_tree1, size_cells, 0 - BEGIN_NODE("subsubnode@0") - PROP_INT(test_tree1, reg, 0) - PROP_INT(test_tree1, phandle, PHANDLE_2) - PROP_STR(test_tree1, compatible, "subsubnode2\0subsubnode") - PROP_INT(test_tree1, prop_int, TEST_VALUE_2) - END_NODE + beginn "subsubnode@0" + propu32 test_tree1, reg, 0 + propu32 test_tree1, phandle, PHANDLE_2 + propstr test_tree1, compatible, "subsubnode2\0subsubnode" + propu32 test_tree1, prop_int, TEST_VALUE_2 + endn - BEGIN_NODE("ss2") - END_NODE + beginn "ss2" + endn - END_NODE + endn - END_NODE - FDTLONG(FDT_END) + endn + fdtlong FDT_END test_tree1_struct_end: test_tree1_strings: - STRING(test_tree1, compatible, "compatible") - STRING(test_tree1, prop_int, "prop-int") - STRING(test_tree1, prop_int64, "prop-int64") - STRING(test_tree1, prop_str, "prop-str") - STRING(test_tree1, linux_phandle, "linux,phandle") - STRING(test_tree1, phandle, "phandle") - STRING(test_tree1, reg, "reg") - STRING(test_tree1, placeholder, "placeholder") - STRING(test_tree1, address_cells, "#address-cells") - STRING(test_tree1, size_cells, "#size-cells") + string test_tree1, compatible, "compatible" + string test_tree1, prop_int, "prop-int" + string test_tree1, prop_int64, "prop-int64" + string test_tree1, prop_str, "prop-str" + string test_tree1, linux_phandle, "linux,phandle" + string test_tree1, phandle, "phandle" + string test_tree1, reg, "reg" + string test_tree1, placeholder, "placeholder" + string test_tree1, address_cells, "#address-cells" + string test_tree1, size_cells, "#size-cells" test_tree1_strings_end: test_tree1_end: - TREE_HDR(truncated_property) - EMPTY_RSVMAP(truncated_property) + treehdr truncated_property + empty_rsvmap truncated_property truncated_property_struct: - BEGIN_NODE("") - PROPHDR(truncated_property, prop_truncated, 4) + beginn "" + prophdr truncated_property, prop_truncated, 4 /* Oops, no actual property data here */ truncated_property_struct_end: truncated_property_strings: - STRING(truncated_property, prop_truncated, "truncated") + string truncated_property, prop_truncated, "truncated" truncated_property_strings_end: truncated_property_end: - TREE_HDR(bad_node_char) - EMPTY_RSVMAP(bad_node_char) + treehdr bad_node_char + empty_rsvmap bad_node_char bad_node_char_struct: - BEGIN_NODE("") - BEGIN_NODE("sub$node") - END_NODE - END_NODE - FDTLONG(FDT_END) + beginn "" + beginn "sub$node" + endn + endn + fdtlong FDT_END bad_node_char_struct_end: bad_node_char_strings: @@ -176,15 +189,15 @@ bad_node_char_strings_end: bad_node_char_end: - TREE_HDR(bad_node_format) - EMPTY_RSVMAP(bad_node_format) + treehdr bad_node_format + empty_rsvmap bad_node_format bad_node_format_struct: - BEGIN_NODE("") - BEGIN_NODE("subnode@1@2") - END_NODE - END_NODE - FDTLONG(FDT_END) + beginn "" + beginn "subnode@1@2" + endn + endn + fdtlong FDT_END bad_node_format_struct_end: bad_node_format_strings: @@ -192,18 +205,18 @@ bad_node_format_strings_end: bad_node_format_end: - TREE_HDR(bad_prop_char) - EMPTY_RSVMAP(bad_prop_char) + treehdr bad_prop_char + empty_rsvmap bad_prop_char bad_prop_char_struct: - BEGIN_NODE("") - PROP_INT(bad_prop_char, prop, TEST_VALUE_1) - END_NODE - FDTLONG(FDT_END) + beginn "" + propu32 bad_prop_char, prop, TEST_VALUE_1 + endn + fdtlong FDT_END bad_prop_char_struct_end: bad_prop_char_strings: - STRING(bad_prop_char, prop, "prop$erty") + string bad_prop_char, prop, "prop$erty" bad_prop_char_strings_end: bad_prop_char_end: @@ -212,46 +225,46 @@ bad_prop_char_end: .balign 8 .globl ovf_size_strings ovf_size_strings: - FDTLONG(FDT_MAGIC) - FDTLONG(ovf_size_strings_end - ovf_size_strings) - FDTLONG(ovf_size_strings_struct - ovf_size_strings) - FDTLONG(ovf_size_strings_strings - ovf_size_strings) - FDTLONG(ovf_size_strings_rsvmap - ovf_size_strings) - FDTLONG(0x11) - FDTLONG(0x10) - FDTLONG(0) - FDTLONG(0xffffffff) - FDTLONG(ovf_size_strings_struct_end - ovf_size_strings_struct) - EMPTY_RSVMAP(ovf_size_strings) + fdtlong FDT_MAGIC + fdtlong (ovf_size_strings_end - ovf_size_strings) + fdtlong (ovf_size_strings_struct - ovf_size_strings) + fdtlong (ovf_size_strings_strings - ovf_size_strings) + fdtlong (ovf_size_strings_rsvmap - ovf_size_strings) + fdtlong 0x11 + fdtlong 0x10 + fdtlong 0 + fdtlong 0xffffffff + fdtlong (ovf_size_strings_struct_end - ovf_size_strings_struct) + empty_rsvmap ovf_size_strings ovf_size_strings_struct: - BEGIN_NODE("") - PROP_INT(ovf_size_strings, bad_string, 0) - END_NODE - FDTLONG(FDT_END) + beginn "" + propu32 ovf_size_strings, bad_string, 0 + endn + fdtlong FDT_END ovf_size_strings_struct_end: ovf_size_strings_strings: - STRING(ovf_size_strings, x, "x") + string ovf_size_strings, x, "x" ovf_size_strings_bad_string = ovf_size_strings_strings + 0x10000000 ovf_size_strings_strings_end: ovf_size_strings_end: /* truncated_string */ - TREE_HDR(truncated_string) - EMPTY_RSVMAP(truncated_string) + treehdr truncated_string + empty_rsvmap truncated_string truncated_string_struct: - BEGIN_NODE("") - PROP_EMPTY(truncated_string, good_string) - PROP_EMPTY(truncated_string, bad_string) - END_NODE - FDTLONG(FDT_END) + beginn "" + propnil truncated_string, good_string + propnil truncated_string, bad_string + endn + fdtlong FDT_END truncated_string_struct_end: truncated_string_strings: - STRING(truncated_string, good_string, "good") + string truncated_string, good_string, "good" truncated_string_bad_string: .ascii "bad" /* NOTE: terminating \0 deliberately missing */ @@ -260,12 +273,12 @@ truncated_string_end: /* truncated_memrsv */ - TREE_HDR(truncated_memrsv) + treehdr truncated_memrsv truncated_memrsv_struct: - BEGIN_NODE("") - END_NODE - FDTLONG(FDT_END) + beginn "" + endn + fdtlong FDT_END truncated_memrsv_struct_end: truncated_memrsv_strings: @@ -273,22 +286,22 @@ truncated_memrsv_strings_end: .balign 8 truncated_memrsv_rsvmap: - RSVMAP_ENTRY(TEST_ADDR_1H, TEST_ADDR_1L, TEST_SIZE_1H, TEST_SIZE_1L) + rsvmape TEST_ADDR_1H, TEST_ADDR_1L, TEST_SIZE_1H, TEST_SIZE_1L truncated_memrsv_rsvmap_end: truncated_memrsv_end: /* two root nodes */ - TREE_HDR(two_roots) - EMPTY_RSVMAP(two_roots) + treehdr two_roots + empty_rsvmap two_roots two_roots_struct: - BEGIN_NODE("") - END_NODE - BEGIN_NODE("") - END_NODE - FDTLONG(FDT_END) + beginn "" + endn + beginn "" + endn + fdtlong FDT_END two_roots_struct_end: two_roots_strings: @@ -298,13 +311,13 @@ two_roots_end: /* root node with a non-empty name */ - TREE_HDR(named_root) - EMPTY_RSVMAP(named_root) + treehdr named_root + empty_rsvmap named_root named_root_struct: - BEGIN_NODE("fake") - END_NODE - FDTLONG(FDT_END) + beginn "fake" + endn + fdtlong FDT_END named_root_struct_end: named_root_strings: From e33ce1d6a8c7e54e3ad12cff33690b6da0aee1dc Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 24 Sep 2021 23:39:56 +1000 Subject: [PATCH 074/104] flattree: Use '\n', not ';' to separate asm pseudo-ops The output of -Oasm is peculiar for assembler in that we want its output to be portable across targets (it consists entirely of pseudo-ops and labels, no actual instructions). It turns out that while ';' is a valid instruction/pseudo-op separator on most targets, it's not correct for all of them - e.g. HP PA-RISC. So, switch to using an actual \n instead. Signed-off-by: David Gibson --- flattree.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/flattree.c b/flattree.c index 5254aeb..95e43d3 100644 --- a/flattree.c +++ b/flattree.c @@ -124,7 +124,8 @@ static void asm_emit_cell(void *e, cell_t val) { FILE *f = e; - fprintf(f, "\t.byte 0x%02x; .byte 0x%02x; .byte 0x%02x; .byte 0x%02x\n", + fprintf(f, "\t.byte\t0x%02x\n" "\t.byte\t0x%02x\n" + "\t.byte\t0x%02x\n" "\t.byte\t0x%02x\n", (val >> 24) & 0xff, (val >> 16) & 0xff, (val >> 8) & 0xff, val & 0xff); } From 37fd700685da9001643d1b1f63ad4332e1148b04 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Tue, 27 Jul 2021 12:30:23 -0600 Subject: [PATCH 075/104] treesource: Maintain phandle label/path on output The dts output will just output phandle integer values, but often the necessary markers are present with path or label references. Improve the output and maintain phandle label or path references when present in dts output. Signed-off-by: Rob Herring Message-Id: <20210727183023.3212077-6-robh@kernel.org> Signed-off-by: David Gibson --- tests/type-preservation.dt.yaml | 3 +++ tests/type-preservation.dts | 3 +++ treesource.c | 25 +++++++++++++++++++------ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/tests/type-preservation.dt.yaml b/tests/type-preservation.dt.yaml index ee8cfde..a0cc64c 100644 --- a/tests/type-preservation.dt.yaml +++ b/tests/type-preservation.dt.yaml @@ -13,8 +13,11 @@ int64: [!u64 [0x200000000]] int64-array: [!u64 [0x100000000, 0x0]] a-string-with-nulls: ["foo\0bar", "baz"] + a-phandle: [[!phandle 0x1]] + a-phandle-with-args: [[!phandle 0x1, 0x0, 0x1], [!phandle 0x1, 0x2, 0x3]] subsubnode: compatible: ["subsubnode1", "subsubnode"] + phandle: [[0x1]] subsubsubnode: compatible: ["subsubsubnode1", [0x1234], "subsubsubnode"] ... diff --git a/tests/type-preservation.dts b/tests/type-preservation.dts index 3e380ba..921ea21 100644 --- a/tests/type-preservation.dts +++ b/tests/type-preservation.dts @@ -16,9 +16,12 @@ int64 = /bits/ 64 <0x200000000>; int64-array = /bits/ 64 <0x100000000 0x00> int64_array_label_end:; a-string-with-nulls = "foo\0bar", "baz"; + a-phandle = <&subsub1>; + a-phandle-with-args = <&subsub1 0x00 0x01>, <&subsub1 0x02 0x03>; subsub1: subsubnode { compatible = "subsubnode1", "subsubnode"; + phandle = <0x01>; subsubsub1: subsubsubnode { compatible = "subsubsubnode1", <0x1234>, valuea: valueb: "subsubsubnode"; diff --git a/treesource.c b/treesource.c index db2ff69..33fedee 100644 --- a/treesource.c +++ b/treesource.c @@ -208,26 +208,39 @@ static void write_propval(FILE *f, struct property *prop) size_t chunk_len = (m->next ? m->next->offset : len) - m->offset; size_t data_len = type_marker_length(m) ? : len - m->offset; const char *p = &prop->val.val[m->offset]; + struct marker *m_phandle; if (is_type_marker(m->type)) { emit_type = m->type; fprintf(f, " %s", delim_start[emit_type]); } else if (m->type == LABEL) fprintf(f, " %s:", m->ref); - else if (m->offset) - fputc(' ', f); - if (emit_type == TYPE_NONE) { - assert(chunk_len == 0); + if (emit_type == TYPE_NONE || chunk_len == 0) continue; - } switch(emit_type) { case TYPE_UINT16: write_propval_int(f, p, chunk_len, 2); break; case TYPE_UINT32: - write_propval_int(f, p, chunk_len, 4); + m_phandle = prop->val.markers; + for_each_marker_of_type(m_phandle, REF_PHANDLE) + if (m->offset == m_phandle->offset) + break; + + if (m_phandle) { + if (m_phandle->ref[0] == '/') + fprintf(f, "&{%s}", m_phandle->ref); + else + fprintf(f, "&%s", m_phandle->ref); + if (chunk_len > 4) { + fputc(' ', f); + write_propval_int(f, p + 4, chunk_len - 4, 4); + } + } else { + write_propval_int(f, p, chunk_len, 4); + } break; case TYPE_UINT64: write_propval_int(f, p, chunk_len, 8); From 52a16fd7282463e05ee86071463bf0499a9cedf6 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Mon, 11 Oct 2021 14:12:41 -0500 Subject: [PATCH 076/104] checks: Make interrupt_provider check dependent on interrupts_extended_is_cell If '#interrupt-cells' doesn't pass checks, no reason to run interrupt provider check. Cc: Andre Przywara Signed-off-by: Rob Herring Message-Id: <20211011191245.1009682-1-robh@kernel.org> Signed-off-by: David Gibson --- checks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checks.c b/checks.c index fb3fc9c..8153793 100644 --- a/checks.c +++ b/checks.c @@ -1587,7 +1587,7 @@ static void check_interrupt_provider(struct check *c, FAIL(c, dti, node, "Missing #address-cells in interrupt provider"); } -WARNING(interrupt_provider, check_interrupt_provider, NULL); +WARNING(interrupt_provider, check_interrupt_provider, NULL, &interrupts_extended_is_cell); static void check_interrupts_property(struct check *c, struct dt_info *dti, From d8d1a9a77863a8c7031ae82a1d461aa78eb72a7b Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Mon, 11 Oct 2021 14:12:43 -0500 Subject: [PATCH 077/104] checks: Drop interrupt provider '#address-cells' check '#address-cells' is only needed when parsing 'interrupt-map' properties, so remove it from the common interrupt-provider test. Cc: Andre Przywara Reviewed-by: David Gibson Signed-off-by: Rob Herring Message-Id: <20211011191245.1009682-3-robh@kernel.org> Signed-off-by: David Gibson --- checks.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/checks.c b/checks.c index 8153793..c6eba76 100644 --- a/checks.c +++ b/checks.c @@ -1581,11 +1581,6 @@ static void check_interrupt_provider(struct check *c, if (!prop) FAIL(c, dti, node, "Missing #interrupt-cells in interrupt provider"); - - prop = get_property(node, "#address-cells"); - if (!prop) - FAIL(c, dti, node, - "Missing #address-cells in interrupt provider"); } WARNING(interrupt_provider, check_interrupt_provider, NULL, &interrupts_extended_is_cell); From 8fd24744e3618be99a939009349418fcbfa362b3 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Mon, 11 Oct 2021 14:12:44 -0500 Subject: [PATCH 078/104] checks: Ensure '#interrupt-cells' only exists in interrupt providers The interrupt provider check currently checks if an interrupt provider has #interrupt-cells, but not whether #interrupt-cells is present outside of interrupt-providers. Rework the check to cover the latter case. Cc: Andre Przywara Reviewed-by: David Gibson Signed-off-by: Rob Herring Message-Id: <20211011191245.1009682-4-robh@kernel.org> Signed-off-by: David Gibson --- checks.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/checks.c b/checks.c index c6eba76..b9fcde8 100644 --- a/checks.c +++ b/checks.c @@ -1573,14 +1573,20 @@ static void check_interrupt_provider(struct check *c, struct node *node) { struct property *prop; - - if (!node_is_interrupt_provider(node)) - return; + bool irq_provider = node_is_interrupt_provider(node); prop = get_property(node, "#interrupt-cells"); - if (!prop) + if (irq_provider && !prop) { FAIL(c, dti, node, - "Missing #interrupt-cells in interrupt provider"); + "Missing '#interrupt-cells' in interrupt provider"); + return; + } + + if (!irq_provider && prop) { + FAIL(c, dti, node, + "'#interrupt-cells' found, but node is not an interrupt provider"); + return; + } } WARNING(interrupt_provider, check_interrupt_provider, NULL, &interrupts_extended_is_cell); From 0a3a9d3449c88aabe88685ea99811bf6c02a570d Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Fri, 15 Oct 2021 16:35:26 -0500 Subject: [PATCH 079/104] checks: Add an interrupt-map check Add a check for parsing 'interrupt-map' properties. The check primarily tests parsing 'interrupt-map' properties which depends on and the parent interrupt controller (or another map) node. Note that this does not require '#address-cells' in the interrupt-map parent, but treats missing '#address-cells' as 0 which is how the Linux kernel parses it. There's numerous cases that expect this behavior. Cc: Andre Przywara Signed-off-by: Rob Herring Message-Id: <20211015213527.2237774-1-robh@kernel.org> Signed-off-by: David Gibson --- checks.c | 85 ++++++++++++++++++++++++++++++ tests/bad-interrupt-map-mask.dts | 20 +++++++ tests/bad-interrupt-map-parent.dts | 17 ++++++ tests/bad-interrupt-map.dts | 19 +++++++ tests/run_tests.sh | 3 ++ 5 files changed, 144 insertions(+) create mode 100644 tests/bad-interrupt-map-mask.dts create mode 100644 tests/bad-interrupt-map-parent.dts create mode 100644 tests/bad-interrupt-map.dts diff --git a/checks.c b/checks.c index b9fcde8..781ba11 100644 --- a/checks.c +++ b/checks.c @@ -1590,6 +1590,90 @@ static void check_interrupt_provider(struct check *c, } WARNING(interrupt_provider, check_interrupt_provider, NULL, &interrupts_extended_is_cell); +static void check_interrupt_map(struct check *c, + struct dt_info *dti, + struct node *node) +{ + struct node *root = dti->dt; + struct property *prop, *irq_map_prop; + size_t cellsize, cell, map_cells; + + irq_map_prop = get_property(node, "interrupt-map"); + if (!irq_map_prop) + return; + + if (node->addr_cells < 0) { + FAIL(c, dti, node, + "Missing '#address-cells' in interrupt-map provider"); + return; + } + cellsize = node_addr_cells(node); + cellsize += propval_cell(get_property(node, "#interrupt-cells")); + + prop = get_property(node, "interrupt-map-mask"); + if (prop && (prop->val.len != (cellsize * sizeof(cell_t)))) + FAIL_PROP(c, dti, node, prop, + "property size (%d) is invalid, expected %zu", + prop->val.len, cellsize * sizeof(cell_t)); + + if (!is_multiple_of(irq_map_prop->val.len, sizeof(cell_t))) { + FAIL_PROP(c, dti, node, irq_map_prop, + "property size (%d) is invalid, expected multiple of %zu", + irq_map_prop->val.len, sizeof(cell_t)); + return; + } + + map_cells = irq_map_prop->val.len / sizeof(cell_t); + for (cell = 0; cell < map_cells; ) { + struct node *provider_node; + struct property *cellprop; + int phandle; + size_t parent_cellsize; + + if ((cell + cellsize) >= map_cells) { + FAIL_PROP(c, dti, node, irq_map_prop, + "property size (%d) too small, expected > %zu", + irq_map_prop->val.len, (cell + cellsize) * sizeof(cell_t)); + break; + } + cell += cellsize; + + phandle = propval_cell_n(irq_map_prop, cell); + if (!phandle_is_valid(phandle)) { + /* Give up if this is an overlay with external references */ + if (!(dti->dtsflags & DTSF_PLUGIN)) + FAIL_PROP(c, dti, node, irq_map_prop, + "Cell %zu is not a phandle(%d)", + cell, phandle); + break; + } + + provider_node = get_node_by_phandle(root, phandle); + if (!provider_node) { + FAIL_PROP(c, dti, node, irq_map_prop, + "Could not get phandle(%d) node for (cell %zu)", + phandle, cell); + break; + } + + cellprop = get_property(provider_node, "#interrupt-cells"); + if (cellprop) { + parent_cellsize = propval_cell(cellprop); + } else { + FAIL(c, dti, node, "Missing property '#interrupt-cells' in node %s or bad phandle (referred from interrupt-map[%zu])", + provider_node->fullpath, cell); + break; + } + + cellprop = get_property(provider_node, "#address-cells"); + if (cellprop) + parent_cellsize += propval_cell(cellprop); + + cell += 1 + parent_cellsize; + } +} +WARNING(interrupt_map, check_interrupt_map, NULL, &phandle_references, &addr_size_cells, &interrupt_provider); + static void check_interrupts_property(struct check *c, struct dt_info *dti, struct node *node) @@ -1888,6 +1972,7 @@ static struct check *check_table[] = { &gpios_property, &interrupts_property, &interrupt_provider, + &interrupt_map, &alias_paths, diff --git a/tests/bad-interrupt-map-mask.dts b/tests/bad-interrupt-map-mask.dts new file mode 100644 index 0000000..10eaffd --- /dev/null +++ b/tests/bad-interrupt-map-mask.dts @@ -0,0 +1,20 @@ +/dts-v1/; + +/ { + interrupt-parent = <&intc>; + intc: interrupt-controller { + #interrupt-cells = <3>; + interrupt-controller; + }; + + node { + #address-cells = <0>; + #interrupt-cells = <1>; + interrupt-map = <1 &intc 1 2 3>; + interrupt-map-mask = <0 0>; + + child { + interrupts = <1>; + }; + }; +}; diff --git a/tests/bad-interrupt-map-parent.dts b/tests/bad-interrupt-map-parent.dts new file mode 100644 index 0000000..fe88ce2 --- /dev/null +++ b/tests/bad-interrupt-map-parent.dts @@ -0,0 +1,17 @@ +/dts-v1/; + +/ { + interrupt-parent = <&intc>; + intc: interrupt-controller { + }; + + node { + #address-cells = <0>; + #interrupt-cells = <1>; + interrupt-map = <1 &intc 1 2 3>; + + child { + interrupts = <1>; + }; + }; +}; diff --git a/tests/bad-interrupt-map.dts b/tests/bad-interrupt-map.dts new file mode 100644 index 0000000..6df8f93 --- /dev/null +++ b/tests/bad-interrupt-map.dts @@ -0,0 +1,19 @@ +/dts-v1/; + +/ { + interrupt-parent = <&intc>; + intc: interrupt-controller { + #interrupt-cells = <3>; + interrupt-controller; + }; + + node { + /* Missing #address-cells = <0>; */ + #interrupt-cells = <1>; + interrupt-map = <1 &intc 1 2 3>; + + child { + interrupts = <1>; + }; + }; +}; diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 0e270fe..d100d5a 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -717,6 +717,9 @@ dtc_tests () { run_sh_test "$SRCDIR/dtc-checkfails.sh" -n deprecated_gpio_property -- -Wdeprecated_gpio_property -I dts -O dtb "$SRCDIR/good-gpio.dts" check_tests "$SRCDIR/bad-interrupt-cells.dts" interrupts_property check_tests "$SRCDIR/bad-interrupt-controller.dts" interrupt_provider + check_tests "$SRCDIR/bad-interrupt-map.dts" interrupt_map + check_tests "$SRCDIR/bad-interrupt-map-parent.dts" interrupt_map + check_tests "$SRCDIR/bad-interrupt-map-mask.dts" interrupt_map run_sh_test "$SRCDIR/dtc-checkfails.sh" node_name_chars -- -I dtb -O dtb bad_node_char.dtb run_sh_test "$SRCDIR/dtc-checkfails.sh" node_name_format -- -I dtb -O dtb bad_node_format.dtb run_sh_test "$SRCDIR/dtc-checkfails.sh" property_name_chars -- -I dtb -O dtb bad_prop_char.dtb From 4eda2590f481db53318a2f203c85eab3dde4b340 Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Sun, 31 Oct 2021 14:09:31 +0100 Subject: [PATCH 080/104] CI: Cirrus: bump used FreeBSD from 12.1 to 13.0 CI freebsd_12 job currently fails to build PRs, because of: ``` ld-elf.so.1: /usr/local/bin/bison: Undefined symbol "fread_unlocked@FBSD_1.6" ``` According to FreeBSD issue tracker[1], the proper solution is to upgrade to a supported release, so do that for our CI. [1]: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=253452 Signed-off-by: Ahmad Fatoum --- .cirrus.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index c270a09..d2dc34e 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -1,9 +1,9 @@ env: CIRRUS_CLONE_DEPTH: 1 -freebsd_12_task: +freebsd_13_task: freebsd_instance: - image: freebsd-12-1-release-amd64 + image: freebsd-13-0-release-amd64 install_script: pkg install -y bison gmake pkgconf build_script: From 5216f3f1bbb70aef463b6cd78dbbb0f4c4c71606 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Wed, 10 Nov 2021 18:33:29 -0600 Subject: [PATCH 081/104] libfdt: Add static lib to meson build The meson build is not building the static libfdt, so add it. Signed-off-by: Rob Herring Message-Id: <20211111003329.2347536-1-robh@kernel.org> Reviewed-by: Simon Glass Tested-by: Simon Glass Signed-off-by: David Gibson --- libfdt/meson.build | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libfdt/meson.build b/libfdt/meson.build index 0307ffb..71f29b6 100644 --- a/libfdt/meson.build +++ b/libfdt/meson.build @@ -24,6 +24,11 @@ libfdt = library( install: true, ) +libfdt_a = static_library( + 'fdt', sources, + install: true, +) + libfdt_inc = include_directories('.') libfdt_dep = declare_dependency( From c691776ddb26acbd3674722caafacaf7b6e3e807 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Wed, 10 Nov 2021 19:11:32 -0600 Subject: [PATCH 082/104] pylibfdt: Use setuptools instead of distutils The use of setuptools is favored over distutils. setuptools is needed to support building Python 'wheels' and for pip support. Signed-off-by: Rob Herring Message-Id: <20211111011135.2386773-2-robh@kernel.org> Signed-off-by: David Gibson --- pylibfdt/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylibfdt/setup.py b/pylibfdt/setup.py index ef40f15..f065a59 100755 --- a/pylibfdt/setup.py +++ b/pylibfdt/setup.py @@ -10,7 +10,7 @@ Copyright (C) 2017 Google, Inc. Written by Simon Glass """ -from distutils.core import setup, Extension +from setuptools import setup, Extension import os import re import sys From 0b106a77dbdc34ef809526febafc490b90d79a54 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Wed, 10 Nov 2021 19:11:33 -0600 Subject: [PATCH 083/104] pylibfdt: Use setuptools_scm for the version The DTC version in version_gen.h causes a warning with setuptools: setuptools/dist.py:501: UserWarning: The version specified ('1.6.1-g5454474d') \ is an invalid version, this may not work as expected with newer versions of \ setuptools, pip, and PyPI. Please see PEP 440 for more details. It also creates an unnecessary dependency on the rest of the build system(s). Switch to use setuptools_scm instead to get the version for pylibfdt. Signed-off-by: Rob Herring Message-Id: <20211111011135.2386773-3-robh@kernel.org> Signed-off-by: David Gibson --- pylibfdt/Makefile.pylibfdt | 2 +- pylibfdt/meson.build | 1 - pylibfdt/setup.py | 18 ++++-------------- 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt index 1b5f236..015a05e 100644 --- a/pylibfdt/Makefile.pylibfdt +++ b/pylibfdt/Makefile.pylibfdt @@ -16,7 +16,7 @@ ifndef V SETUPFLAGS += --quiet endif -$(PYMODULE): $(PYLIBFDT_srcs) $(LIBFDT_archive) $(SETUP) $(VERSION_FILE) +$(PYMODULE): $(PYLIBFDT_srcs) $(LIBFDT_archive) $(SETUP) @$(VECHO) PYMOD $@ $(PYTHON) $(SETUP) $(SETUPFLAGS) build_ext --build-lib=$(PYLIBFDT_dir) diff --git a/pylibfdt/meson.build b/pylibfdt/meson.build index 088f249..fad5aa1 100644 --- a/pylibfdt/meson.build +++ b/pylibfdt/meson.build @@ -5,7 +5,6 @@ custom_target( 'pylibfdt', input: 'libfdt.i', output: '_libfdt.so', - depends: version_gen_h, command: [setup_py, 'build_ext', '--build-lib=' + meson.current_build_dir()], build_by_default: true, ) diff --git a/pylibfdt/setup.py b/pylibfdt/setup.py index f065a59..52b61b6 100755 --- a/pylibfdt/setup.py +++ b/pylibfdt/setup.py @@ -15,10 +15,6 @@ import os import re import sys - -VERSION_PATTERN = '^#define DTC_VERSION "DTC ([^"]*)"$' - - def get_top_builddir(): if '--top-builddir' in sys.argv: index = sys.argv.index('--top-builddir') @@ -27,18 +23,9 @@ def get_top_builddir(): else: return os.getcwd() - srcdir = os.path.dirname(os.path.abspath(sys.argv[0])) top_builddir = get_top_builddir() - -def get_version(): - version_file = os.path.join(top_builddir, 'version_gen.h') - f = open(version_file, 'rt') - m = re.match(VERSION_PATTERN, f.readline()) - return m.group(1) - - libfdt_module = Extension( '_libfdt', sources=[os.path.join(srcdir, 'libfdt.i')], @@ -50,7 +37,10 @@ libfdt_module = Extension( setup( name='libfdt', - version=get_version(), + use_scm_version={ + "root": os.path.join(srcdir, '..'), + }, + setup_requires = ['setuptools_scm'], author='Simon Glass ', description='Python binding for libfdt', ext_modules=[libfdt_module], From 69a760747d8d9d1c5dcceebc05e868e1eaf13a2b Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Wed, 10 Nov 2021 19:11:34 -0600 Subject: [PATCH 084/104] pylibfdt: Split setup.py author name and email The 'author' field in setup.py is supposed to be just the name. The email address goes in 'author_email' field. Cc: Simon Glass Signed-off-by: Rob Herring Message-Id: <20211111011135.2386773-4-robh@kernel.org> Signed-off-by: David Gibson --- pylibfdt/setup.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pylibfdt/setup.py b/pylibfdt/setup.py index 52b61b6..75ce09a 100755 --- a/pylibfdt/setup.py +++ b/pylibfdt/setup.py @@ -41,7 +41,8 @@ setup( "root": os.path.join(srcdir, '..'), }, setup_requires = ['setuptools_scm'], - author='Simon Glass ', + author='Simon Glass', + author_email='sjg@chromium.org', description='Python binding for libfdt', ext_modules=[libfdt_module], package_dir={'': srcdir}, From 23b56cb7e18992650c79a04c9e4e3f2740bc1fbd Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Wed, 10 Nov 2021 19:11:35 -0600 Subject: [PATCH 085/104] pylibfdt: Move setup.py to the top level Using 'pip' and several setup.py sub-commands currently don't work with pylibfdt. The primary reason is Python packaging has opinions on the directory structure of repositories and one of those appears to be the inability to reference source files outside of setup.py's subtree. This means a sdist cannot be created with all necessary source components (i.e. libfdt headers). Moving setup.py to the top-level solves these problems. With this change. the following commands now work: Creating packages for pypi.org: ./setup.py sdist bdist_wheel Using pip for installs: pip install . pip install git+http://github.com/robherring/dtc.git@pypi-v2 Signed-off-by: Rob Herring Message-Id: <20211111011135.2386773-5-robh@kernel.org> Signed-off-by: David Gibson --- .gitignore | 4 ++++ MANIFEST.in | 9 +++++++++ pylibfdt/Makefile.pylibfdt | 3 +-- pylibfdt/meson.build | 4 ++-- pylibfdt/setup.py => setup.py | 15 ++++++++------- 5 files changed, 24 insertions(+), 11 deletions(-) create mode 100644 MANIFEST.in rename pylibfdt/setup.py => setup.py (75%) diff --git a/.gitignore b/.gitignore index 8e332d8..416fa05 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,7 @@ lex.yy.c # cscope files cscope.* ncscope.* + +.eggs/ +build/ +dist/ diff --git a/MANIFEST.in b/MANIFEST.in new file mode 100644 index 0000000..9e6c4ac --- /dev/null +++ b/MANIFEST.in @@ -0,0 +1,9 @@ +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) + +global-exclude * +include setup.py +include pylibfdt/libfdt.i +include pylibfdt/*.py +include libfdt/libfdt.h +include libfdt/fdt.h +include libfdt/libfdt_env.h diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt index 015a05e..82f565e 100644 --- a/pylibfdt/Makefile.pylibfdt +++ b/pylibfdt/Makefile.pylibfdt @@ -9,8 +9,7 @@ PYLIBFDT_CLEANFILES = $(PYLIBFDT_CLEANFILES_L:%=$(PYLIBFDT_dir)/%) PYLIBFDT_CLEANDIRS_L = build __pycache__ PYLIBFDT_CLEANDIRS = $(PYLIBFDT_CLEANDIRS_L:%=$(PYLIBFDT_dir)/%) -SETUP = $(PYLIBFDT_dir)/setup.py -SETUPFLAGS = --top-builddir . +SETUP = ./setup.py ifndef V SETUPFLAGS += --quiet diff --git a/pylibfdt/meson.build b/pylibfdt/meson.build index fad5aa1..f684cbb 100644 --- a/pylibfdt/meson.build +++ b/pylibfdt/meson.build @@ -1,5 +1,5 @@ -setup_py = find_program('setup.py') -setup_py = [setup_py.path(), '--quiet', '--top-builddir', meson.current_build_dir() / '..'] +setup_py = find_program('../setup.py') +setup_py = [setup_py.path(), '--quiet', '--top-builddir', meson.project_build_root()] custom_target( 'pylibfdt', diff --git a/pylibfdt/setup.py b/setup.py similarity index 75% rename from pylibfdt/setup.py rename to setup.py index 75ce09a..4b07be9 100755 --- a/pylibfdt/setup.py +++ b/setup.py @@ -15,36 +15,37 @@ import os import re import sys +srcdir = os.path.dirname(__file__) + def get_top_builddir(): if '--top-builddir' in sys.argv: index = sys.argv.index('--top-builddir') sys.argv.pop(index) return sys.argv.pop(index) else: - return os.getcwd() + return srcdir -srcdir = os.path.dirname(os.path.abspath(sys.argv[0])) top_builddir = get_top_builddir() libfdt_module = Extension( '_libfdt', - sources=[os.path.join(srcdir, 'libfdt.i')], - include_dirs=[os.path.join(srcdir, '../libfdt')], + sources=[os.path.join(srcdir, 'pylibfdt/libfdt.i')], + include_dirs=[os.path.join(srcdir, 'libfdt')], libraries=['fdt'], library_dirs=[os.path.join(top_builddir, 'libfdt')], - swig_opts=['-I' + os.path.join(srcdir, '../libfdt')], + swig_opts=['-I' + os.path.join(srcdir, 'libfdt')], ) setup( name='libfdt', use_scm_version={ - "root": os.path.join(srcdir, '..'), + "root": srcdir, }, setup_requires = ['setuptools_scm'], author='Simon Glass', author_email='sjg@chromium.org', description='Python binding for libfdt', ext_modules=[libfdt_module], - package_dir={'': srcdir}, + package_dir={'': os.path.join(srcdir, 'pylibfdt')}, py_modules=['libfdt'], ) From 383e148b70a47ab15f97a19bb999d54f9c3e810f Mon Sep 17 00:00:00 2001 From: Ross Burton Date: Thu, 11 Nov 2021 16:05:36 +0000 Subject: [PATCH 086/104] pylibfdt: fix with Python 3.10 Since Python 2.5 the argument parsing functions when parsing expressions such as s# (string plus length) expect the length to be an int or a ssize_t, depending on whether PY_SSIZE_T_CLEAN is defined or not. Python 3.8 deprecated the use of int, and with Python 3.10 this symbol must be defined and ssize_t used[1]. Define the magic symbol when building the extension, and cast the ints from the libfdt API to ssize_t as appropriate. [1] https://docs.python.org/3.10/whatsnew/3.10.html#id2 Signed-off-by: Ross Burton Message-Id: <20211111160536.2516573-1-ross.burton@arm.com> [dwg: Adjust for new location of setup.py] Tested-by: Rob Herring Signed-off-by: David Gibson --- pylibfdt/libfdt.i | 4 ++-- setup.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i index 51ee801..075ef70 100644 --- a/pylibfdt/libfdt.i +++ b/pylibfdt/libfdt.i @@ -1044,9 +1044,9 @@ typedef uint32_t fdt32_t; $result = Py_None; else %#if PY_VERSION_HEX >= 0x03000000 - $result = Py_BuildValue("y#", $1, *arg4); + $result = Py_BuildValue("y#", $1, (Py_ssize_t)*arg4); %#else - $result = Py_BuildValue("s#", $1, *arg4); + $result = Py_BuildValue("s#", $1, (Py_ssize_t)*arg4); %#endif } diff --git a/setup.py b/setup.py index 4b07be9..0a0daf1 100755 --- a/setup.py +++ b/setup.py @@ -30,6 +30,7 @@ top_builddir = get_top_builddir() libfdt_module = Extension( '_libfdt', sources=[os.path.join(srcdir, 'pylibfdt/libfdt.i')], + define_macros=[('PY_SSIZE_T_CLEAN', None)], include_dirs=[os.path.join(srcdir, 'libfdt')], libraries=['fdt'], library_dirs=[os.path.join(top_builddir, 'libfdt')], From db72398cd4371324901e8ff54980543e0139c1b9 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Thu, 11 Nov 2021 22:16:28 -0600 Subject: [PATCH 087/104] README: Update pylibfdt install instructions Now that pip is supported for installs, update the install instructions to use it. Using pip over setup.py is generally recommended and simpler. Also, drop 'SETUP_PREFIX' as it doesn't exist anywhere. Signed-off-by: Rob Herring Message-Id: <20211112041633.741598-2-robh@kernel.org> Signed-off-by: David Gibson --- README | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/README b/README index d9bf850..5f5655d 100644 --- a/README +++ b/README @@ -48,18 +48,24 @@ If you add new features, please check code coverage: # Open 'htmlcov/index.html' in your browser -To install the library via the normal setup.py method, use: +The library can be installed with pip from a local source tree: - ./pylibfdt/setup.py install [--prefix=/path/to/install_dir] + pip install . [--user|--prefix=/path/to/install_dir] -If --prefix is not provided, the default prefix is used, typically '/usr' -or '/usr/local'. See Python's distutils documentation for details. You can -also install via the Makefile if you like, but the above is more common. +Or directly from a remote git repo: + + pip install git+git://git.kernel.org/pub/scm/utils/dtc/dtc.git@main + +The install depends on libfdt shared library being installed on the host system +first. Generally, using --user or --prefix is not necessary and pip will use the +default location for the Python installation which varies if the user is root or +not. + +You can also install everything via make if you like, but pip is recommended. To install both libfdt and pylibfdt you can use: - make install [SETUP_PREFIX=/path/to/install_dir] \ - [PREFIX=/path/to/install_dir] + make install [PREFIX=/path/to/install_dir] To disable building the python library, even if swig and Python are available, use: From 1cc41b1c969f1fa5090b166397e4bab4ab1aa449 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Thu, 11 Nov 2021 22:16:29 -0600 Subject: [PATCH 088/104] pylibfdt: Add packaging metadata PyPI expects to have various package metadata including long description, license, and classifiers. Add them. Signed-off-by: Rob Herring Message-Id: <20211112041633.741598-3-robh@kernel.org> Signed-off-by: David Gibson --- MANIFEST.in | 3 +++ setup.py | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/MANIFEST.in b/MANIFEST.in index 9e6c4ac..ff8f5d6 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,6 +1,9 @@ # SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) global-exclude * +include README +include GPL +include BSD-2-Clause include setup.py include pylibfdt/libfdt.i include pylibfdt/*.py diff --git a/setup.py b/setup.py index 0a0daf1..00444e6 100755 --- a/setup.py +++ b/setup.py @@ -17,6 +17,9 @@ import sys srcdir = os.path.dirname(__file__) +with open("README", "r") as fh: + long_description = fh.read() + def get_top_builddir(): if '--top-builddir' in sys.argv: index = sys.argv.index('--top-builddir') @@ -49,4 +52,18 @@ setup( ext_modules=[libfdt_module], package_dir={'': os.path.join(srcdir, 'pylibfdt')}, py_modules=['libfdt'], + + long_description=long_description, + long_description_content_type="text/plain", + url="https://git.kernel.org/pub/scm/utils/dtc/dtc.git", + license="BSD", + license_files=["GPL", "BSD-2-Clause"], + + classifiers=[ + "Programming Language :: Python :: 3", + "License :: OSI Approved :: BSD License", + "License :: OSI Approved :: GNU General Public License v2 or later (GPLv2+)", + "Operating System :: OS Independent", + ], + ) From c19a4bafa514c2c85876f0de35782abd11bed7ec Mon Sep 17 00:00:00 2001 From: Elvira Khabirova Date: Tue, 9 Nov 2021 18:47:19 +0300 Subject: [PATCH 089/104] libfdt: fix an incorrect integer promotion UINT32_MAX is an integer of type unsigned int. UINT32_MAX + 1 overflows unless explicitly computed as unsigned long long. This led to some invalid addresses being treated as valid. Cast UINT32_MAX to uint64_t explicitly. Signed-off-by: Elvira Khabirova --- libfdt/fdt_addresses.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfdt/fdt_addresses.c b/libfdt/fdt_addresses.c index 9a82cd0..c40ba09 100644 --- a/libfdt/fdt_addresses.c +++ b/libfdt/fdt_addresses.c @@ -73,7 +73,7 @@ int fdt_appendprop_addrrange(void *fdt, int parent, int nodeoffset, /* check validity of address */ prop = data; if (addr_cells == 1) { - if ((addr > UINT32_MAX) || ((UINT32_MAX + 1 - addr) < size)) + if ((addr > UINT32_MAX) || (((uint64_t) UINT32_MAX + 1 - addr) < size)) return -FDT_ERR_BADVALUE; fdt32_st(prop, (uint32_t)addr); From 45f3d1a095dd3440578d5c6313eba555a791f3fb Mon Sep 17 00:00:00 2001 From: Vikram Garhwal Date: Wed, 17 Nov 2021 18:53:56 -0800 Subject: [PATCH 090/104] libfdt: overlay: make overlay_get_target() public This is done to get the target path for the overlay nodes which is very useful in many cases. For example, Xen hypervisor needs it when applying overlays because Xen needs to do further processing of the overlay nodes, e.g. mapping of resources(IRQs and IOMMUs) to other VMs, creation of SMMU pagetables, etc. Signed-off-by: Vikram Garhwal Message-Id: <1637204036-382159-2-git-send-email-fnu.vikram@xilinx.com> Signed-off-by: David Gibson --- libfdt/fdt_overlay.c | 29 +++++++---------------------- libfdt/libfdt.h | 18 ++++++++++++++++++ libfdt/version.lds | 1 + 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c index d217e79..5c0c398 100644 --- a/libfdt/fdt_overlay.c +++ b/libfdt/fdt_overlay.c @@ -40,37 +40,22 @@ static uint32_t overlay_get_target_phandle(const void *fdto, int fragment) return fdt32_to_cpu(*val); } -/** - * overlay_get_target - retrieves the offset of a fragment's target - * @fdt: Base device tree blob - * @fdto: Device tree overlay blob - * @fragment: node offset of the fragment in the overlay - * @pathp: pointer which receives the path of the target (or NULL) - * - * overlay_get_target() retrieves the target offset in the base - * device tree of a fragment, no matter how the actual targeting is - * done (through a phandle or a path) - * - * returns: - * the targeted node offset in the base device tree - * Negative error code on error - */ -static int overlay_get_target(const void *fdt, const void *fdto, - int fragment, char const **pathp) +int fdt_overlay_target_offset(const void *fdt, const void *fdto, + int fragment_offset, char const **pathp) { uint32_t phandle; const char *path = NULL; int path_len = 0, ret; /* Try first to do a phandle based lookup */ - phandle = overlay_get_target_phandle(fdto, fragment); + phandle = overlay_get_target_phandle(fdto, fragment_offset); if (phandle == (uint32_t)-1) return -FDT_ERR_BADPHANDLE; /* no phandle, try path */ if (!phandle) { /* And then a path based lookup */ - path = fdt_getprop(fdto, fragment, "target-path", &path_len); + path = fdt_getprop(fdto, fragment_offset, "target-path", &path_len); if (path) ret = fdt_path_offset(fdt, path); else @@ -636,7 +621,7 @@ static int overlay_merge(void *fdt, void *fdto) if (overlay < 0) return overlay; - target = overlay_get_target(fdt, fdto, fragment, NULL); + target = fdt_overlay_target_offset(fdt, fdto, fragment, NULL); if (target < 0) return target; @@ -779,7 +764,7 @@ static int overlay_symbol_update(void *fdt, void *fdto) return -FDT_ERR_BADOVERLAY; /* get the target of the fragment */ - ret = overlay_get_target(fdt, fdto, fragment, &target_path); + ret = fdt_overlay_target_offset(fdt, fdto, fragment, &target_path); if (ret < 0) return ret; target = ret; @@ -801,7 +786,7 @@ static int overlay_symbol_update(void *fdt, void *fdto) if (!target_path) { /* again in case setprop_placeholder changed it */ - ret = overlay_get_target(fdt, fdto, fragment, &target_path); + ret = fdt_overlay_target_offset(fdt, fdto, fragment, &target_path); if (ret < 0) return ret; target = ret; diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h index 7f117e8..a7f432c 100644 --- a/libfdt/libfdt.h +++ b/libfdt/libfdt.h @@ -2116,6 +2116,24 @@ int fdt_del_node(void *fdt, int nodeoffset); */ int fdt_overlay_apply(void *fdt, void *fdto); +/** + * fdt_overlay_target_offset - retrieves the offset of a fragment's target + * @fdt: Base device tree blob + * @fdto: Device tree overlay blob + * @fragment_offset: node offset of the fragment in the overlay + * @pathp: pointer which receives the path of the target (or NULL) + * + * fdt_overlay_target_offset() retrieves the target offset in the base + * device tree of a fragment, no matter how the actual targeting is + * done (through a phandle or a path) + * + * returns: + * the targeted node offset in the base device tree + * Negative error code on error + */ +int fdt_overlay_target_offset(const void *fdt, const void *fdto, + int fragment_offset, char const **pathp); + /**********************************************************************/ /* Debugging / informational functions */ /**********************************************************************/ diff --git a/libfdt/version.lds b/libfdt/version.lds index 7ab85f1..cbce5d4 100644 --- a/libfdt/version.lds +++ b/libfdt/version.lds @@ -77,6 +77,7 @@ LIBFDT_1.2 { fdt_appendprop_addrrange; fdt_setprop_inplace_namelen_partial; fdt_create_with_flags; + fdt_overlay_target_offset; local: *; }; From 17739b7ef510917471409d71fb45d8eaf6a1e1fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Mi=C5=82ecki?= Date: Thu, 9 Dec 2021 07:14:20 +0100 Subject: [PATCH 091/104] Support 'r' format for printing raw bytes with fdtget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FT is sometimes used for storing raw data. That is quite common for U-Boot FIT images. Extracting such data is not trivial currently. Using type 's' (string) will replace every 0x00 (NUL) with 0x20 (space). Using type 'x' will print bytes but in xxd incompatible format. This commit adds support for 'r' (raw) format. Example usage: fdtget -t r firmware.itb /images/foo data > image.raw Support for encoding isn't added as there isn't any clean way of passing binary data as command line argument. Signed-off-by: Rafał Miłecki Message-Id: <20211209061420.29466-1-zajec5@gmail.com> Signed-off-by: David Gibson --- Documentation/manual.txt | 2 +- fdtget.c | 5 +++++ fdtput.c | 2 ++ tests/run_tests.sh | 2 ++ tests/utilfdt_test.c | 5 ++++- util.c | 4 ++-- util.h | 3 ++- 7 files changed, 18 insertions(+), 5 deletions(-) diff --git a/Documentation/manual.txt b/Documentation/manual.txt index 97e53b9..cf4b253 100644 --- a/Documentation/manual.txt +++ b/Documentation/manual.txt @@ -712,7 +712,7 @@ The syntax of the fdtget command is: where options are: - s=string, i=int, u=unsigned, x=hex + s=string, i=int, u=unsigned, x=hex, r=raw Optional modifier prefix: hh or b=byte, h=2 byte, l=4 byte (default) diff --git a/fdtget.c b/fdtget.c index 54fc6a0..dd70985 100644 --- a/fdtget.c +++ b/fdtget.c @@ -97,6 +97,11 @@ static int show_data(struct display_info *disp, const char *data, int len) if (len == 0) return 0; + if (disp->type == 'r') { + fwrite(data, 1, len, stdout); + return 0; + } + is_string = (disp->type) == 's' || (!disp->type && util_is_printable_string(data, len)); if (is_string) { diff --git a/fdtput.c b/fdtput.c index 428745a..c2fecf4 100644 --- a/fdtput.c +++ b/fdtput.c @@ -433,6 +433,8 @@ int main(int argc, char *argv[]) if (utilfdt_decode_type(optarg, &disp.type, &disp.size)) usage("Invalid type string"); + if (disp.type == 'r') + usage("Unsupported raw data type"); break; case 'v': diff --git a/tests/run_tests.sh b/tests/run_tests.sh index d100d5a..11068e1 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -855,6 +855,8 @@ fdtget_tests () { run_fdtget_test 8000 -tx $dtb /cpus/PowerPC,970@1 d-cache-size run_fdtget_test "61 62 63 0" -tbx $dtb /randomnode tricky1 run_fdtget_test "a b c d de ea ad be ef" -tbx $dtb /randomnode blob + run_fdtget_test "MyBoardName\0MyBoardFamilyName\0" -tr $dtb / compatible + run_fdtget_test "\x0a\x0b\x0c\x0d\xde\xea\xad\xbe\xef" -tr $dtb /randomnode blob # Here the property size is not a multiple of 4 bytes, so it should fail run_wrap_error_test $DTGET -tlx $dtb /randomnode mixed diff --git a/tests/utilfdt_test.c b/tests/utilfdt_test.c index c621759..ba6462f 100644 --- a/tests/utilfdt_test.c +++ b/tests/utilfdt_test.c @@ -73,6 +73,9 @@ static void check_sizes(char *modifier, int expected_size) *ptr = 's'; check(fmt, 's', -1); + + *ptr = 'r'; + check(fmt, 'r', -1); } static void test_utilfdt_decode_type(void) @@ -90,7 +93,7 @@ static void test_utilfdt_decode_type(void) /* try every other character */ checkfail(""); for (ch = ' '; ch < 127; ch++) { - if (!strchr("iuxs", ch)) { + if (!strchr("iuxsr", ch)) { *fmt = ch; fmt[1] = '\0'; checkfail(fmt); diff --git a/util.c b/util.c index 40274fb..14d3868 100644 --- a/util.c +++ b/util.c @@ -353,11 +353,11 @@ int utilfdt_decode_type(const char *fmt, int *type, int *size) } /* we should now have a type */ - if ((*fmt == '\0') || !strchr("iuxs", *fmt)) + if ((*fmt == '\0') || !strchr("iuxsr", *fmt)) return -1; /* convert qualifier (bhL) to byte size */ - if (*fmt != 's') + if (*fmt != 's' && *fmt != 'r') *size = qualifier == 'b' ? 1 : qualifier == 'h' ? 2 : qualifier == 'l' ? 4 : -1; diff --git a/util.h b/util.h index c45b2c2..7a4e910 100644 --- a/util.h +++ b/util.h @@ -143,6 +143,7 @@ int utilfdt_write_err(const char *filename, const void *blob); * i signed integer * u unsigned integer * x hex + * r raw * * TODO: Implement ll modifier (8 bytes) * TODO: Implement o type (octal) @@ -160,7 +161,7 @@ int utilfdt_decode_type(const char *fmt, int *type, int *size); */ #define USAGE_TYPE_MSG \ - "\ts=string, i=int, u=unsigned, x=hex\n" \ + "\ts=string, i=int, u=unsigned, x=hex, r=raw\n" \ "\tOptional modifier prefix:\n" \ "\t\thh or b=byte, h=2 byte, l=4 byte (default)"; From d152126bb0293c321cae437bdf7437c393ee3619 Mon Sep 17 00:00:00 2001 From: Luca Weiss Date: Fri, 24 Dec 2021 11:28:12 +0100 Subject: [PATCH 092/104] Fix Python crash on getprop deallocation Fatal Python error: none_dealloc: deallocating None Python runtime state: finalizing (tstate=0x000055c9bac70920) Current thread 0x00007fbe34e47740 (most recent call first): Aborted (core dumped) This is caused by a missing Py_INCREF on the returned Py_None, as demonstrated e.g. in https://github.com/mythosil/swig-python-incref or described at https://edcjones.tripod.com/refcount.html ("Remember to INCREF Py_None!") A PoC for triggering this crash is uploaded to https://github.com/z3ntu/pylibfdt-crash . With this patch applied to pylibfdt the crash does not happen. Signed-off-by: Luca Weiss Message-Id: <20211224102811.70695-1-luca@z3ntu.xyz> Reviewed-by: Simon Glass Signed-off-by: David Gibson --- pylibfdt/libfdt.i | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i index 075ef70..9ccc57b 100644 --- a/pylibfdt/libfdt.i +++ b/pylibfdt/libfdt.i @@ -1040,14 +1040,16 @@ typedef uint32_t fdt32_t; /* typemap used for fdt_getprop() */ %typemap(out) (const void *) { - if (!$1) + if (!$1) { $result = Py_None; - else + Py_INCREF($result); + } else { %#if PY_VERSION_HEX >= 0x03000000 $result = Py_BuildValue("y#", $1, (Py_ssize_t)*arg4); %#else $result = Py_BuildValue("s#", $1, (Py_ssize_t)*arg4); %#endif + } } /* typemap used for fdt_setprop() */ From 83102717d7c4171aeb2d26941fa1ee2997bf4a7d Mon Sep 17 00:00:00 2001 From: Luca Weiss Date: Sat, 25 Dec 2021 14:25:55 +0100 Subject: [PATCH 093/104] pylibfdt: add Property.as_stringlist() Add a new method for decoding a string list property, useful for e.g. the "reg-names" property. Also add a test for the new method. Signed-off-by: Luca Weiss Message-Id: <20211225132558.167123-2-luca@z3ntu.xyz> Signed-off-by: David Gibson --- pylibfdt/libfdt.i | 7 +++++++ tests/pylibfdt_tests.py | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i index 9ccc57b..c81b504 100644 --- a/pylibfdt/libfdt.i +++ b/pylibfdt/libfdt.i @@ -724,6 +724,13 @@ class Property(bytearray): raise ValueError('Property contains embedded nul characters') return self[:-1].decode('utf-8') + def as_stringlist(self): + """Unicode is supported by decoding from UTF-8""" + if self[-1] != 0: + raise ValueError('Property lacks nul termination') + parts = self[:-1].split(b'\x00') + return list(map(lambda x: x.decode('utf-8'), parts)) + class FdtSw(FdtRo): """Software interface to create a device tree from scratch diff --git a/tests/pylibfdt_tests.py b/tests/pylibfdt_tests.py index 64b5bd1..7e3cc4c 100644 --- a/tests/pylibfdt_tests.py +++ b/tests/pylibfdt_tests.py @@ -382,6 +382,14 @@ class PyLibfdtBasicTests(unittest.TestCase): self.get_prop("prop-uint64").as_uint64()) self.assertEqual(-2, self.get_prop("prop-int64").as_int64()) + def testGetStringlistProperties(self): + """Test that we can access properties as string list""" + node = self.fdt.path_offset('/subnode@1/subsubnode') + self.assertEqual(["subsubnode1", "subsubnode"], + self.fdt.getprop(node, "compatible").as_stringlist()) + self.assertEqual(["this is a placeholder string", "string2"], + self.fdt.getprop(node, "placeholder").as_stringlist()) + def testReserveMap(self): """Test that we can access the memory reserve map""" self.assertEqual(2, self.fdt.num_mem_rsv()) From a04f69025003890be265e007238dc91041e5529b Mon Sep 17 00:00:00 2001 From: Luca Weiss Date: Sat, 25 Dec 2021 14:25:56 +0100 Subject: [PATCH 094/104] pylibfdt: add Property.as_*int*_array() Add new methods to handle decoding of int32, uint32, int64 and uint64 arrays. Also add tests for the new methods. Signed-off-by: Luca Weiss Message-Id: <20211225132558.167123-3-luca@z3ntu.xyz> Signed-off-by: David Gibson --- pylibfdt/libfdt.i | 15 +++++++++++++++ tests/pylibfdt_tests.py | 11 +++++++++++ tests/test_props.dts | 4 ++++ 3 files changed, 30 insertions(+) diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i index c81b504..ac70762 100644 --- a/pylibfdt/libfdt.i +++ b/pylibfdt/libfdt.i @@ -716,6 +716,21 @@ class Property(bytearray): def as_int64(self): return self.as_cell('q') + def as_list(self, fmt): + return list(map(lambda x: x[0], struct.iter_unpack('>' + fmt, self))) + + def as_uint32_list(self): + return self.as_list('L') + + def as_int32_list(self): + return self.as_list('l') + + def as_uint64_list(self): + return self.as_list('Q') + + def as_int64_list(self): + return self.as_list('q') + def as_str(self): """Unicode is supported by decoding from UTF-8""" if self[-1] != 0: diff --git a/tests/pylibfdt_tests.py b/tests/pylibfdt_tests.py index 7e3cc4c..5479363 100644 --- a/tests/pylibfdt_tests.py +++ b/tests/pylibfdt_tests.py @@ -382,6 +382,17 @@ class PyLibfdtBasicTests(unittest.TestCase): self.get_prop("prop-uint64").as_uint64()) self.assertEqual(-2, self.get_prop("prop-int64").as_int64()) + def testGetIntListProperties(self): + """Test that we can access properties as integer lists""" + self.assertEqual([128, -16, -2], + self.get_prop("prop-int32-array").as_int32_list()) + self.assertEqual([0x1, 0x98765432, 0xdeadbeef], + self.get_prop("prop-uint32-array").as_uint32_list()) + self.assertEqual([0x100000000, -2], + self.get_prop("prop-int64-array").as_int64_list()) + self.assertEqual([0x100000000, 0x1], + self.get_prop("prop-uint64-array").as_uint64_list()) + def testGetStringlistProperties(self): """Test that we can access properties as string list""" node = self.fdt.path_offset('/subnode@1/subsubnode') diff --git a/tests/test_props.dts b/tests/test_props.dts index 7e59bd1..5089023 100644 --- a/tests/test_props.dts +++ b/tests/test_props.dts @@ -8,4 +8,8 @@ prop-hex64 = /bits/ 64 <0xdeadbeef01abcdef>; prop-uint64 = /bits/ 64 <9223372036854775807>; prop-int64 = /bits/ 64 <0xfffffffffffffffe>; + prop-int32-array = <128>, <(-16)>, <0xfffffffe>; + prop-uint32-array = <0x1>, <0x98765432>, <0xdeadbeef>; + prop-int64-array = /bits/ 64 <0x100000000 0xfffffffffffffffe>; + prop-uint64-array = /bits/ 64 <0x100000000 0x1>; }; From cd5f69cbc0d4bc34a509b5f6f62234e25893b684 Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Thu, 9 Dec 2021 09:58:00 +0100 Subject: [PATCH 095/104] tests: setprop_inplace: use xstrdup instead of unchecked strdup This is the only strdup instance we have, all others are xstrdup. As strdup is _POSIX_C_SOURCE >= v200809L, which we don't require and we don't check strdup error return here, switch to xstrdup instead. This aligns the test with others that call xfuncs, mainly xmalloc(). Signed-off-by: Ahmad Fatoum Signed-off-by: David Gibson --- tests/setprop_inplace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/setprop_inplace.c b/tests/setprop_inplace.c index 7e1198d..61a0da1 100644 --- a/tests/setprop_inplace.c +++ b/tests/setprop_inplace.c @@ -58,7 +58,7 @@ int main(int argc, char *argv[]) TEST_STRING_1); verbose_printf("Old string value was \"%s\"\n", strp); - xstr = strdup(strp); + xstr = xstrdup(strp); xlen = strlen(xstr); for (i = 0; i < xlen; i++) xstr[i] = toupper(xstr[i]); From c0c2e115f82ed3bc5f9d3f9e5380f0f7e81a1c21 Mon Sep 17 00:00:00 2001 From: LoveSy Date: Wed, 15 Dec 2021 17:30:11 +0800 Subject: [PATCH 096/104] Fix a UB when fdt_get_string return null When fdt_get_string return null, `namep` is not correctly reset. From the document of `fdt_getprop_by_offset`, the parameter `namep` will be always overwritten (that is, it will be overwritten without exception of error occurance). As for the caller (like https://github.com/topjohnwu/Magisk/blob/e097c097feb881f6097b6d1dc346f310bc92f5d6/native/jni/magiskboot/dtb.cpp#L42), the code may be like: ```cpp size_t size; const char *name; auto *value = fdt_getprop_by_offset(fdt, prop, &name, &size); ``` and if `value == nullptr`, `size` is also be overwritten correctly but `name` is not, which is quite inconsistent. This commit makes sure `name` and `size` behavior consistently (reset to reasonable value) when error occurs. Signed-off-by: LoveSy Signed-off-by: David Gibson --- libfdt/fdt_ro.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c index 17584da..9f6c551 100644 --- a/libfdt/fdt_ro.c +++ b/libfdt/fdt_ro.c @@ -481,12 +481,12 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset, if (!can_assume(VALID_INPUT)) { name = fdt_get_string(fdt, fdt32_ld_(&prop->nameoff), &namelen); + *namep = name; if (!name) { if (lenp) *lenp = namelen; return NULL; } - *namep = name; } else { *namep = fdt_string(fdt, fdt32_ld_(&prop->nameoff)); } From ca7294434309930076ff99ff4f0c817499f50f5a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 7 Nov 2021 15:43:43 -0700 Subject: [PATCH 097/104] README: Explain how to add a new API function This is not obvious so add a little note about it. Signed-off-by: Simon Glass Message-Id: <20211107224346.3181320-2-sjg@chromium.org> Signed-off-by: David Gibson --- README | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/README b/README index 5f5655d..a48312a 100644 --- a/README +++ b/README @@ -77,6 +77,15 @@ More work remains to support all of libfdt, including access to numeric values. +Adding a new function to libfdt.h +--------------------------------- + +The shared library uses libfdt/version.lds to list the exported functions, so +add your new function there. Check that your function works with pylibfdt. If +it cannot be supported, put the declaration in libfdt.h behind #ifndef SWIG so +that swig ignores it. + + Tests ----- From ff5afb96d0c08133b6f709a197b8bda023531757 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 29 Dec 2021 15:08:28 +1100 Subject: [PATCH 098/104] Handle integer overflow in check_property_phandle_args() If the corresponding '#xxx-cells' value is much too large, an integer overflow can prevent the checks in check_property_phandle_args() from correctly determining that the checked property is too short for the given cells value. This leads to an infinite loops. This patch fixes the bug, and adds a testcase for it. Further information in https://github.com/dgibson/dtc/issues/64 Reported-by: Anciety Signed-off-by: David Gibson --- checks.c | 15 +++++++++------ tests/phandle-args-overflow.dts | 18 ++++++++++++++++++ tests/run_tests.sh | 3 +++ 3 files changed, 30 insertions(+), 6 deletions(-) create mode 100644 tests/phandle-args-overflow.dts diff --git a/checks.c b/checks.c index 781ba11..9f31d26 100644 --- a/checks.c +++ b/checks.c @@ -1382,10 +1382,10 @@ struct provider { }; static void check_property_phandle_args(struct check *c, - struct dt_info *dti, - struct node *node, - struct property *prop, - const struct provider *provider) + struct dt_info *dti, + struct node *node, + struct property *prop, + const struct provider *provider) { struct node *root = dti->dt; unsigned int cell, cellsize = 0; @@ -1401,6 +1401,7 @@ static void check_property_phandle_args(struct check *c, struct node *provider_node; struct property *cellprop; cell_t phandle; + unsigned int expected; phandle = propval_cell_n(prop, cell); /* @@ -1450,10 +1451,12 @@ static void check_property_phandle_args(struct check *c, break; } - if (prop->val.len < ((cell + cellsize + 1) * sizeof(cell_t))) { + expected = (cell + cellsize + 1) * sizeof(cell_t); + if ((expected <= cell) || prop->val.len < expected) { FAIL_PROP(c, dti, node, prop, - "property size (%d) too small for cell size %d", + "property size (%d) too small for cell size %u", prop->val.len, cellsize); + break; } } } diff --git a/tests/phandle-args-overflow.dts b/tests/phandle-args-overflow.dts new file mode 100644 index 0000000..8c8c5d7 --- /dev/null +++ b/tests/phandle-args-overflow.dts @@ -0,0 +1,18 @@ +/dts-v1/; + +/* + * https://github.com/dgibson/dtc/issues/64 + * + * Certain dtc versions had a bug where this input caused an infinite + * loop in check_property_phandle_args(). + * + */ + +/ { + clocks = <&ref &ref>; + + ref: poc { + phandle = <1>; + #clock-cells = <0xffffffff>; + }; +}; diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 11068e1..5e4e7c4 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -513,6 +513,9 @@ libfdt_tests () { run_dtc_test -I fs -O dtb -o fs.test_tree1.test.dtb $FSBASE/test_tree1 run_test dtbs_equal_unordered -m fs.test_tree1.test.dtb test_tree1.dtb + ## https://github.com/dgibson/dtc/issues/64 + check_tests "$SRCDIR/phandle-args-overflow.dts" clocks_property + # check full tests for good in test_tree1.dtb; do run_test check_full $good From 4048aed12b81c5a0154b9af438edc99ec7d2b6a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 3 Jan 2022 11:38:55 +0400 Subject: [PATCH 099/104] setup.py: fix out of tree build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: commit 1cc41b1c9 ("pylibfdt: Add packaging metadata") Signed-off-by: Marc-André Lureau Message-Id: <20220103073855.1468799-3-marcandre.lureau@redhat.com> Signed-off-by: David Gibson --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 00444e6..029aa61 100755 --- a/setup.py +++ b/setup.py @@ -17,7 +17,7 @@ import sys srcdir = os.path.dirname(__file__) -with open("README", "r") as fh: +with open(os.path.join(srcdir, "README"), "r") as fh: long_description = fh.read() def get_top_builddir(): From 651410e54cb9a478f2fbb16cb493a5e7808ad8fe Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Sun, 25 Oct 2020 10:41:18 +0100 Subject: [PATCH 100/104] util: introduce xstrndup helper We already have xstrdup, add xstrndup as well to make it straight-forward to clone part of a string. Signed-off-by: Ahmad Fatoum --- util.c | 11 +++++++++++ util.h | 1 + 2 files changed, 12 insertions(+) diff --git a/util.c b/util.c index 14d3868..507f012 100644 --- a/util.c +++ b/util.c @@ -33,6 +33,17 @@ char *xstrdup(const char *s) return d; } +char *xstrndup(const char *s, size_t n) +{ + size_t len = strnlen(s, n) + 1; + char *d = xmalloc(len); + + memcpy(d, s, len - 1); + d[len - 1] = '\0'; + + return d; +} + int xavsprintf_append(char **strp, const char *fmt, va_list ap) { int n, size = 0; /* start with 128 bytes */ diff --git a/util.h b/util.h index 7a4e910..9d38ede 100644 --- a/util.h +++ b/util.h @@ -61,6 +61,7 @@ static inline void *xrealloc(void *p, size_t len) } extern char *xstrdup(const char *s); +extern char *xstrndup(const char *s, size_t len); extern int PRINTF(2, 3) xasprintf(char **strp, const char *fmt, ...); extern int PRINTF(2, 3) xasprintf_append(char **strp, const char *fmt, ...); From ec7986e682cf9a5078bdbad341feb807a56b21e0 Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Sun, 25 Oct 2020 12:39:59 +0100 Subject: [PATCH 101/104] dtc: introduce label relative path references Reference via label allows extending nodes with compile-time checking of whether the node being extended exists. This is useful to catch renamed/removed nodes after an update of the device trees to be extended. In absence of labels in the original device trees, new style path references can be used: /* upstream device tree */ / { leds: some-non-standard-led-controller-name { led-0 { default-state = "off"; }; }; }; /* downstream device tree */ &{/some-non-standard-led-controller-name/led-0} { default-state = "on"; }; This is a common theme within the barebox bootloader[0], which extends the upstream (Linux) device trees in that manner. The downside is that, especially for deep nodes, these references can get quite long and tend to break often due to upstream rework (e.g. rename to adhere to bindings). Often there is a label a level or two higher that could be used. This patch allows combining both a label and a new style path reference to get a compile-time-checked reference, which allows rewriting the previous downstream device tree snippet to: &{leds/led-0} { default-state = "on"; }; This won't be broken when /some-non-standard-led-controller-name is renamed or moved while keeping the label. And if led-0 is renamed, we will get the expected compile-time error. Overlay support is skipped for now as they require special support: The label and relative path parts need to be resolved at overlay apply-time, not at compile-time. [0]: https://www.barebox.org/doc/latest/devicetree/index.html Signed-off-by: Ahmad Fatoum --- dtc-lexer.l | 2 +- dtc-parser.y | 13 +++++++++++++ livetree.c | 33 ++++++++++++++++++++++++++++++--- 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/dtc-lexer.l b/dtc-lexer.l index 5568b4a..de60a70 100644 --- a/dtc-lexer.l +++ b/dtc-lexer.l @@ -200,7 +200,7 @@ static void PRINTF(1, 2) lexical_error(const char *fmt, ...); return DT_LABEL_REF; } -<*>"&{/"{PATHCHAR}*\} { /* new-style path reference */ +<*>"&{"{PATHCHAR}*\} { /* new-style path reference */ yytext[yyleng-1] = '\0'; DPRINT("Ref: %s\n", yytext+2); yylval.labelref = xstrdup(yytext+2); diff --git a/dtc-parser.y b/dtc-parser.y index a0316a3..46457d4 100644 --- a/dtc-parser.y +++ b/dtc-parser.y @@ -23,6 +23,12 @@ extern void yyerror(char const *s); extern struct dt_info *parser_output; extern bool treesource_error; + +static bool is_ref_relative(const char *ref) +{ + return ref[0] != '/' && strchr(&ref[1], '/'); +} + %} %union { @@ -169,6 +175,8 @@ devicetree: */ if (!($-1 & DTSF_PLUGIN)) ERROR(&@2, "Label or path %s not found", $1); + else if (is_ref_relative($1)) + ERROR(&@2, "Label-relative reference %s not supported in plugin", $1); $$ = add_orphan_node( name_node(build_node(NULL, NULL, NULL), ""), @@ -178,6 +186,9 @@ devicetree: { struct node *target = get_node_by_ref($1, $3); + if (($-1 & DTSF_PLUGIN) && is_ref_relative($3)) + ERROR(&@2, "Label-relative reference %s not supported in plugin", $3); + if (target) { add_label(&target->labels, $2); merge_nodes(target, $4); @@ -193,6 +204,8 @@ devicetree: * so $-1 is what we want (plugindecl) */ if ($-1 & DTSF_PLUGIN) { + if (is_ref_relative($2)) + ERROR(&@2, "Label-relative reference %s not supported in plugin", $2); add_orphan_node($1, $3, $2); } else { struct node *target = get_node_by_ref($1, $2); diff --git a/livetree.c b/livetree.c index cc61237..169462d 100644 --- a/livetree.c +++ b/livetree.c @@ -581,12 +581,39 @@ struct node *get_node_by_phandle(struct node *tree, cell_t phandle) struct node *get_node_by_ref(struct node *tree, const char *ref) { + struct node *target = tree; + const char *label = NULL, *path = NULL; + if (streq(ref, "/")) return tree; - else if (ref[0] == '/') - return get_node_by_path(tree, ref); + + if (ref[0] == '/') + path = ref; else - return get_node_by_label(tree, ref); + label = ref; + + if (label) { + const char *slash = strchr(label, '/'); + char *buf = NULL; + + if (slash) { + buf = xstrndup(label, slash - label); + label = buf; + path = slash + 1; + } + + target = get_node_by_label(tree, label); + + free(buf); + + if (!target) + return NULL; + } + + if (path) + target = get_node_by_path(target, path); + + return target; } cell_t get_node_phandle(struct node *root, struct node *node) From 26c54f840d2340271f305c04f0d66bafac93274f Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Sun, 25 Oct 2020 10:41:41 +0100 Subject: [PATCH 102/104] tests: add test cases for label-relative path references Newly added &{label/path} feature doesn't yet have any tests. Add some. Signed-off-by: Ahmad Fatoum --- tests/.gitignore | 1 + tests/Makefile.tests | 2 +- tests/path-references.c | 8 +++++- tests/path-references.dts | 9 +++++++ tests/relref_merge.c | 51 +++++++++++++++++++++++++++++++++++++++ tests/relref_merge.dts | 40 ++++++++++++++++++++++++++++++ tests/run_tests.sh | 3 +++ 7 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 tests/relref_merge.c create mode 100644 tests/relref_merge.dts diff --git a/tests/.gitignore b/tests/.gitignore index d3f1434..03bdde2 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -54,6 +54,7 @@ tmp.* /property_iterate /propname_escapes /references +/relref_merge /root_node /rw_tree1 /rw_oom diff --git a/tests/Makefile.tests b/tests/Makefile.tests index 2b47627..2f78952 100644 --- a/tests/Makefile.tests +++ b/tests/Makefile.tests @@ -18,7 +18,7 @@ LIB_TESTS_L = get_mem_rsv \ open_pack rw_tree1 rw_oom set_name setprop del_property del_node \ appendprop1 appendprop2 propname_escapes \ string_escapes references path-references phandle_format \ - boot-cpuid incbin \ + boot-cpuid incbin relref_merge \ extra-terminating-null \ dtbs_equal_ordered \ dtb_reverse dtbs_equal_unordered \ diff --git a/tests/path-references.c b/tests/path-references.c index 4db61a5..b914576 100644 --- a/tests/path-references.c +++ b/tests/path-references.c @@ -53,7 +53,7 @@ int main(int argc, char *argv[]) void *fdt; const char *p; int len, multilen; - int n1, n2, n3, n4; + int n1, n2, n3, n4, n5; test_init(argc, argv); fdt = load_blob_arg(argc, argv); @@ -89,6 +89,12 @@ int main(int argc, char *argv[]) check_ref(fdt, n3, "/foobar/baz"); check_ref(fdt, n4, "/foo/baz"); + n5 = fdt_path_offset(fdt, "/bar/baz"); + if (n5 < 0) + FAIL("fdt_path_offset(/bar/baz): %s", fdt_strerror(n5)); + + check_ref(fdt, n5, "/bar/baz"); + check_rref(fdt); PASS(); diff --git a/tests/path-references.dts b/tests/path-references.dts index 1fb7d70..dd9ccfc 100644 --- a/tests/path-references.dts +++ b/tests/path-references.dts @@ -25,4 +25,13 @@ lref = &n3; }; }; + n5: bar { + baz { + }; + }; +}; + +n6: &{n5/baz} { + ref = &{n6/}; + lref = &{n5/baz}; }; diff --git a/tests/relref_merge.c b/tests/relref_merge.c new file mode 100644 index 0000000..5daab2e --- /dev/null +++ b/tests/relref_merge.c @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later +/* + * libfdt - Flat Device Tree manipulation + * Testcase for label relative child references in dtc + * Copyright (C) 2006 David Gibson, IBM Corporation. + * Copyright (C) 2020 Ahmad Fatoum, Pengutronix. + */ +#include +#include +#include +#include + +#include + +#include "tests.h" +#include "testdata.h" + +static void check_exist(void *fdt, const char *path) +{ + int sn = fdt_path_offset(fdt, path); + if (sn < 0) + FAIL("%s expected but not found: %s", path, fdt_strerror(sn)); +} + +static void check_doesnt_exist(void *fdt, const char *path) +{ + int sn = fdt_path_offset(fdt, path); + if (sn >= 0) + FAIL("%s found but not expected %d", path, sn); +} + +int main(int argc, char *argv[]) +{ + void *fdt; + + test_init(argc, argv); + fdt = load_blob_arg(argc, argv); + + check_exist(fdt, "/node/subnode1"); + check_exist(fdt, "/node/keep-me"); + check_doesnt_exist(fdt, "/node/remove-me"); + + check_doesnt_exist(fdt, "/node2"); + check_doesnt_exist(fdt, "/node/subnode3"); + + check_exist(fdt, "/node/subnode4"); + + check_exist(fdt, "/node/subnode1/add-me"); + + PASS(); +} diff --git a/tests/relref_merge.dts b/tests/relref_merge.dts new file mode 100644 index 0000000..d8660eb --- /dev/null +++ b/tests/relref_merge.dts @@ -0,0 +1,40 @@ +/dts-v1/; + +/ { + node_label: node { + keep-me {}; + remove-me {}; + + subnode1 { + property-inline1; + property-inline2; + property-inline3; + }; + + subnode2 { + property-inline1; + }; + + subnode3 { + property-inline1; + }; + }; + + node2_label: node2 { + property-inline1; + }; +}; +/omit-if-no-ref/ &{node_label/subnode1}; +/omit-if-no-ref/ &node2_label; +/delete-node/ &{node_label/subnode3}; + +&{node_label/} { + /delete-node/ remove-me; + + subnode4 { }; +}; + +label: &{node_label/subnode1} { + selfref = &{node_label/subnode1}; + add-me { }; +}; diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 5e4e7c4..c78351d 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -669,6 +669,9 @@ dtc_tests () { tree1_tests dtc_tree1_merge_path.test.dtb test_tree1.dtb run_wrap_error_test $DTC -I dts -O dtb -o /dev/null "$SRCDIR/test_label_ref.dts" + run_dtc_test -I dts -O dtb -o dtc_relref_merge.test.dtb "$SRCDIR/relref_merge.dts" + run_test relref_merge dtc_relref_merge.test.dtb + # Check prop/node delete functionality run_dtc_test -I dts -O dtb -o dtc_tree1_delete.test.dtb "$SRCDIR/test_tree1_delete.dts" tree1_tests dtc_tree1_delete.test.dtb From c001fc01a43e7a06447c06ea3d50bd60641322b8 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Thu, 3 Feb 2022 12:04:07 -0600 Subject: [PATCH 103/104] pylibfdt: fix swig build in install A 'pip install' is silently broken unless the tree is dirty and contains pylibfdt/libfdt.py. The problem is a known issue[1] with SWIG and setuptools where the 'build_py' stage needing module.py runs before the 'build_ext' stage which generates it. The work-around is to override 'build_py' to run 'build_ext' first. [1] https://stackoverflow.com/questions/50239473/building-a-module-with-setuptools-and-swig Signed-off-by: Rob Herring Message-Id: <20220203180408.611645-2-robh@kernel.org> Signed-off-by: David Gibson --- MANIFEST.in | 1 - setup.py | 8 ++++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/MANIFEST.in b/MANIFEST.in index ff8f5d6..0eee931 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -6,7 +6,6 @@ include GPL include BSD-2-Clause include setup.py include pylibfdt/libfdt.i -include pylibfdt/*.py include libfdt/libfdt.h include libfdt/fdt.h include libfdt/libfdt_env.h diff --git a/setup.py b/setup.py index 029aa61..a8e54a3 100755 --- a/setup.py +++ b/setup.py @@ -11,6 +11,8 @@ Written by Simon Glass """ from setuptools import setup, Extension +from setuptools.command.build_py import build_py as _build_py + import os import re import sys @@ -40,11 +42,17 @@ libfdt_module = Extension( swig_opts=['-I' + os.path.join(srcdir, 'libfdt')], ) +class build_py(_build_py): + def run(self): + self.run_command("build_ext") + return super().run() + setup( name='libfdt', use_scm_version={ "root": srcdir, }, + cmdclass = {'build_py' : build_py}, setup_requires = ['setuptools_scm'], author='Simon Glass', author_email='sjg@chromium.org', From ed310803ea893ed0a8bba9c4ff0d9eb0063a8bef Mon Sep 17 00:00:00 2001 From: Luca Weiss Date: Tue, 19 Apr 2022 21:45:38 +0200 Subject: [PATCH 104/104] pylibfdt: add FdtRo.get_path() Add a new Python method wrapping fdt_get_path() from the C API. Also add a test for the new method. Signed-off-by: Luca Weiss Message-Id: <20220419194537.63170-1-luca@z3ntu.xyz> Reviewed-by: Simon Glass Signed-off-by: David Gibson --- pylibfdt/libfdt.i | 28 ++++++++++++++++++++++++++++ tests/pylibfdt_tests.py | 13 +++++++++++++ 2 files changed, 41 insertions(+) diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i index ac70762..f9f7e7e 100644 --- a/pylibfdt/libfdt.i +++ b/pylibfdt/libfdt.i @@ -443,6 +443,29 @@ class FdtRo(object): """ return fdt_get_alias(self._fdt, name) + def get_path(self, nodeoffset, quiet=()): + """Get the full path of a node + + Args: + nodeoffset: Node offset to check + + Returns: + Full path to the node + + Raises: + FdtException if an error occurs + """ + size = 1024 + while True: + ret, path = fdt_get_path(self._fdt, nodeoffset, size) + if ret == -NOSPACE: + size = size * 2 + continue + err = check_err(ret, quiet) + if err: + return err + return path + def parent_offset(self, nodeoffset, quiet=()): """Get the offset of a node's parent @@ -1115,6 +1138,11 @@ typedef uint32_t fdt32_t; } } +%include "cstring.i" + +%cstring_output_maxsize(char *buf, int buflen); +int fdt_get_path(const void *fdt, int nodeoffset, char *buf, int buflen); + /* We have both struct fdt_property and a function fdt_property() */ %warnfilter(302) fdt_property; diff --git a/tests/pylibfdt_tests.py b/tests/pylibfdt_tests.py index 5479363..68d6aaa 100644 --- a/tests/pylibfdt_tests.py +++ b/tests/pylibfdt_tests.py @@ -348,6 +348,19 @@ class PyLibfdtBasicTests(unittest.TestCase): self.assertEqual("/subnode@1/subsubnode", self.fdt3.get_alias('ss1')) self.assertEqual("/subnode@1/subsubnode/subsubsubnode", self.fdt3.get_alias('sss1')) + def testGetPath(self): + """Test for the get_path() method""" + node = self.fdt.path_offset('/subnode@1') + node2 = self.fdt.path_offset('/subnode@1/subsubnode') + self.assertEqual("/subnode@1", self.fdt.get_path(node)) + self.assertEqual("/subnode@1/subsubnode", self.fdt.get_path(node2)) + + with self.assertRaises(FdtException) as e: + self.fdt.get_path(-1) + self.assertEqual(e.exception.err, -libfdt.BADOFFSET) + + self.assertEqual(-libfdt.BADOFFSET, self.fdt.get_path(-1, quiet=(libfdt.BADOFFSET,))) + def testParentOffset(self): """Test for the parent_offset() method""" self.assertEqual(-libfdt.NOTFOUND,