[U-Boot] __FILE__ usage and and SPL limits for SRAM

[Tom Rini <trini@konsulko.com>]

On Tue, Mar 28, 2017 at 11:37:45AM +0530, Lokesh Vutla wrote:
> + more folks.
> 
> On Tuesday 28 March 2017 03:14 AM, Nishanth Menon wrote:
> > Hi,
> > 
> > we've kind of run into an interesting situation recently, but might be
> > of interest for various folks trying to reduce the image sizes.
> > 
> > our AM335x device has a limited amount of sram.. and the SPL tries to
> > fit into it (a bit tricky given the restricted space we have on it on
> > certain class of devices).
> > 
> > arch/arm/mach-omap2/am33xx/u-boot-spl.lds is a bit custom tailored
> > around this.
> > 
> > Key in this is:
> > . = ALIGN(4);
> > .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram
> > 
> > . = ALIGN(4);
> > .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram
> > 
> > 
> > Now, our jenkins build system happens to use a varied build path and
> > uses O= path. to simplify the details:
> > mkdir
> > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc
> > 
> > mkdir
> > /tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/cccccccccccccccccccccccccccccccccccccccccccccccccc/b
> > 
> > 
> > git clone u-boot
> > cd u-boot
> > 
> > git clean -fdx
> > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b am335x_evm_defconfig
> > make CROSS_COMPILE=arm-linux-gnueabihf- O=../b all
> > 
> > depending on depth of the path, this would fail.. a little bit of
> > headscratching later..
> > when using O= build system uses absolute paths, which translates to
> > __FILE__ being absolute paths as well..
> > 
> > in u-boot, any printf("%s", __FILE__) makes u-boot allocate this file
> > path in rodata.
> > 
> > So, depending on how deep the path is rodata size varies and ends up
> > pushing .data out of sram max range.
> > 
> > we dont really care to put a print of complete absolute path anyways,
> > and I am not really sure of a clean way to resolve this:
> > a) override __FILE__ with something.. -Wbuiltin-macro-redefined kicks in
> > b) replace usage of __FILE__ with something like __FILENAME__ as
> > recommended by [1]
> > 
> > 
> > What is the suggestion we do?
> > 
> > [1] http://stackoverflow.com/questions/8487986/file-macro-shows-full-path
> 
> Any suggestions would be really helpful.

So here's what I've come up with, and it's slightly incomplete:
diff --git a/include/common.h b/include/common.h
index 2cbbd5a60cd3..cdc61ef5b144 100644
--- a/include/common.h
+++ b/include/common.h
@@ -145,20 +145,19 @@ typedef volatile unsigned char	vu_char;
  * in any case these failing assertions cannot be fixed with a reset (which
  * may just do the same assertion again).
  */
-void __assert_fail(const char *assertion, const char *file, unsigned line,
-		   const char *function);
+void __assert_fail(const char *assertion, unsigned line, const char *function);
 #define assert(x) \
 	({ if (!(x) && _DEBUG) \
-		__assert_fail(#x, __FILE__, __LINE__, __func__); })
+		__assert_fail(#x, __LINE__, __func__); })
 
 #define error(fmt, args...) do {					\
-		printf("ERROR: " pr_fmt(fmt) "\nat %s:%d/%s()\n",	\
-			##args, __FILE__, __LINE__, __func__);		\
+		printf("ERROR: " pr_fmt(fmt) "\nat %s():%d\n",	\
+			##args, __func__, __LINE__);		\
 } while (0)
 
 #ifndef BUG
 #define BUG() do { \
-	printf("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \
+	printf("BUG: failure at %s():%d!\n", __FUNCTION__, __LINE__); \
 	panic("BUG!"); \
 } while (0)
 #define BUG_ON(condition) do { if (unlikely((condition)!=0)) BUG(); } while(0)
diff --git a/include/image.h b/include/image.h
index 237251896029..a52c5355376e 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1218,12 +1218,12 @@ static inline int fit_image_check_target_arch(const void *fdt, int node)
 #ifdef CONFIG_FIT_VERBOSE
 #define fit_unsupported(msg)	printf("! %s:%d " \
 				"FIT images not supported for '%s'\n", \
-				__FILE__, __LINE__, (msg))
+				__func__, __LINE__, (msg))
 
 #define fit_unsupported_reset(msg)	printf("! %s:%d " \
 				"FIT images not supported for '%s' " \
 				"- must reset board to recover!\n", \
-				__FILE__, __LINE__, (msg))
+				__func__, __LINE__, (msg))
 #else
 #define fit_unsupported(msg)
 #define fit_unsupported_reset(msg)
diff --git a/include/linux/compat.h b/include/linux/compat.h
index a43e4d66983d..db9fcb6d47c2 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -100,14 +100,14 @@ static inline void kmem_cache_destroy(struct kmem_cache *cachep)
 
 #ifndef BUG
 #define BUG() do { \
-	printf("U-Boot BUG at %s:%d!\n", __FILE__, __LINE__); \
+	printf("U-Boot BUG at %s:%d!\n", __func__, __LINE__); \
 } while (0)
 
 #define BUG_ON(condition) do { if (condition) BUG(); } while(0)
 #endif /* BUG */
 
 #define WARN_ON(x) if (x) {printf("WARNING in %s line %d\n" \
-				  , __FILE__, __LINE__); }
+				  , __func__, __LINE__); }
 
 #define PAGE_SIZE	4096
 
diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
index 6def8f98aa41..b620463174d7 100644
--- a/lib/tiny-printf.c
+++ b/lib/tiny-printf.c
@@ -228,11 +228,9 @@ int snprintf(char *buf, size_t size, const char *fmt, ...)
 	return ret;
 }
 
-void __assert_fail(const char *assertion, const char *file, unsigned line,
-		   const char *function)
+void __assert_fail(const char *assertion, unsigned line, const char *function)
 {
 	/* This will not return */
-	printf("%s:%u: %s: Assertion `%s' failed.", file, line, function,
-	       assertion);
+	printf("%s:%u: Assertion `%s' failed.", function, line, assertion);
 	hang();
 }
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 874a2951f705..d14cd67b3817 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -722,12 +722,10 @@ int vprintf(const char *fmt, va_list args)
 }
 
 
-void __assert_fail(const char *assertion, const char *file, unsigned line,
-		   const char *function)
+void __assert_fail(const char *assertion, unsigned line, const char *function)
 {
 	/* This will not return */
-	panic("%s:%u: %s: Assertion `%s' failed.", file, line, function,
-	      assertion);
+	panic("%s:%u: Assertion `%s' failed.", function, line, assertion);
 }
 
 char *simple_itoa(ulong i)

Why?  I'm not sure that in most cases __FILE__ is providing any more
useful infomration on top of what we have from __func__ and __LINE__.
That said, converting __assert_fail is probably not a good idea as it'll
lead to cases (probably) where we don't have enough context to know
where the problem really was, but with everything else being a macro it
should evaluate out as useful as before at least.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170404/468e54c0/attachment.sig>