[PATCH v1 0/7 SECTOR: Support aligning to flash sector boundary

[PATCH v1 0/7 SECTOR: Support aligning to flash sector boundary

[binutils]

Patchset 1

Introduce the BANK command which allows
the user to define flash sector boundaries.

I.E:

BANK (bankname) {
  SECTOR ("32 KB");
  SECTOR ("32 KB");
  SECTOR ("64 KB");
  SECTOR ("64 KB");
  SECTOR ("64 KB");
}

This can be used later to align a sector to the flash boundary using the 

ALIGN_SECTOR command in an output sector.

A user who want to use their own tool to create the
a checksum, maybe using a SHA algorithm need a
simple way to understand the limits of the area
to calculate the digest on.
By aligning to a flash sector, this is much simplified.

When writing a new image to the flash, you first have
to erase the flash sector, so it is practical
to align stuff to flash sector boundaries.

[PATCH v1 1/7] SECTOR: NEWS
[PATCH v1 2/7] SECTOR: ld.texi
[PATCH v1 3/7] SECTOR: ldlex.l
[PATCH v1 4/7] SECTOR: ldgram.y
[PATCH v1 5/7] SECTOR: language additions
[PATCH v1 6/7] SECTOR: add testsuite
[PATCH v1 7/7] SECTOR: Makefile.*

[PATCH v1 1/7] SECTOR: NEWS

[binutils]

From: Ulf Samuelsson <ulf@emagii.com>

Signed-off-by: Ulf Samuelsson <ulf@emagii.com>
---
 ld/NEWS | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/ld/NEWS b/ld/NEWS
index b79023ecb95..21b33b58220 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -1,5 +1,47 @@
 -*- text -*-
 
+* The linker script now support aligning to a Flash sector boundary.
+  The BANK command defines the flash bank name, and contains a list
+  of sector definitions.
+  symbols are defined for "begin", "end" and "size" of a sector.
+
+  An 'ALIGN_SECTOR' command will check the location counter
+  against the defined sectors and if inside a sector, it will
+  align to the sector boundary.
+
+  New commands:
+
+  BANK ( bankname ) { sector_list }
+  SECTOR ( "<size" ) ;  - sector definition: valid inside a BANK command
+  ALIGN_SECTOR
+
+  The size parameter is a string containing two elements.
+  A number and a modifier. Valid modifiers are "BYTES", "KB" and "MB".
+  The modifiers are not case sensitive.
+
+  Example:
+
+  BANK ( bank0 ) {
+    SECTOR ("1 KB");
+    SECTOR ("1 KB");
+    SECTOR ("2 KB");
+  }
+
+  SECTIONS
+  {
+    .text : {
+	sector0 = .;
+	ASCIZ "sector0"
+        endstring = .;
+	ALIGN_SECTOR
+	sector1 = .;
+    }
+  }
+
+  sector0   == 0x0000
+  endstring == 0x0008
+  sector1   == 0x0400
+
 * The linker script has a new command to insert a timestamp
   TIMESTAMP
   inserts the current time (seconds since Epoch) as a 64-bit value
-- 
2.34.1

[PATCH v1 2/7] SECTOR: ld.texi

[binutils]

From: Ulf Samuelsson <ulf@emagii.com>

Signed-off-by: Ulf Samuelsson <ulf@emagii.com>
---
 ld/ld.texi | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/ld/ld.texi b/ld/ld.texi
index c21d849fe24..4b4af5fb46b 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -5884,6 +5884,12 @@ calc_crc32 (const unsigned char *input_str, size_t num_bytes)
 The @code{TIMESTAMP} command creates 64-bit integer with the number of seconds
 since Epoch (1970-01-01 00:00).
 
+@cindex output section strings
+@kindex ALIGN_SECTOR
+
+The @code{ALIGN_SECTOR} command will align the location counter to the next
+flash sector boundary. The sectors are defined by a @code{BANK} command.
+
 @kindex FILL(@var{expression})
 @cindex holes, filling
 @cindex unspecified memory
@@ -6574,7 +6580,104 @@ expression via the @code{ORIGIN(@var{memory})} and
   _fstack = ORIGIN(ram) + LENGTH(ram) - 4;
 @end group
 @end smallexample
+@page
+@node BANK
+@section BANK Command
+@kindex BANK
+@kindex SECTOR
+@cindex memory regions
+@cindex regions of memory
+@cindex allocating memory
+
+The @code{BANK} command defines the boundaries of flash memory sectors.
+It contains a list of @code{SECTOR} commands, each defining symbols for the
+@code{begin}, @code{end} and @code{size} of a sector prefixed by the bank name.
+You can have multiple banks. The first bank starts at address @code{0x00000000}.
+A bank with a name that starts with @code{dummy} is ignored and can be used
+as a filler.
+
+The @code{SECTOR} command has a single string parameter containing the size
+of the sector. It can contain a multiplier. Valid multipliers are
+@code{BYTES}, @code{KB} and @code{MB} for a multiplier of 1, 1024 and 1024*1024.
+@sp 1
+Example of @code{BANK} command defining 3 flash sectors.
+@sp 1
+@smallexample
+@group
+  BANK ( bank0 ) @{
+    SECTOR ("16 KB");
+    SECTOR ("16 KB");
+    SECTOR ("32 KB");
+  @}
+@end group
+@end smallexample
+
+@sp 1
+Example of a @code{BANK} command defining 3 flash sectors combined with
+@code{ALIGN_SECTOR} command.
+@sp 1
+@smallexample
+@group
+  BANK ( bank0 ) @{
+    SECTOR ("1 KB");
+    SECTOR ("1 KB");
+    SECTOR ("2 KB");
+  @}
+
+  SECTIONS
+  @{
+    .text : @{
+        sector0 = .;
+        ASCIZ "sector0"
+        endstring = .;
+        ALIGN_SECTOR
+        sector1 = .;
+    @}
+  @}
+
+  sector0   == 0x0000
+  endstring == 0x0008
+  sector1   == 0x0400
+@end group
+@end smallexample
 
+@page
+
+Example of using a dummy bank to shift the origin to 0x10000.
+@sp 1
+@smallexample
+@group
+  BANK ( dummy ) @{
+    SECTOR ("64 KB");
+  @}
+  BANK ( bank0 ) @{
+    SECTOR ("1 KB");
+    SECTOR ("1 KB");
+    SECTOR ("2 KB");
+  @}
+  MEMORY @{
+    rom : ORIGIN = 0x10000, LENGTH = 0x4000
+    ram : ORIGIN = 0x14000, LENGTH = 0x1000
+  @}
+  SECTIONS
+  @{
+    .text : @{
+        sector0 = .;
+        ASCIZ "sector0"
+        endstring = .;
+        ALIGN_SECTOR
+        sector1 = .;
+    @} > rom
+  @}
+
+  sector0   == 0x10000
+  endstring == 0x10008
+  sector1   == 0x10400
+@end group
+@end smallexample
+
+
+@page
 @node PHDRS
 @section PHDRS Command
 @kindex PHDRS
-- 
2.34.1

[PATCH v1 3/7] SECTOR: ldlex.l

[binutils]

From: Ulf Samuelsson <ulf@emagii.com>

Add new keyboards

* "BANK"
* "SECTOR"
* "ALIGN_SECTOR"

Signed-off-by: Ulf Samuelsson <ulf@emagii.com>
---
 ld/ldlex.l | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ld/ldlex.l b/ld/ldlex.l
index a240538be6b..3b3d1fb8c8b 100644
--- a/ld/ldlex.l
+++ b/ld/ldlex.l
@@ -318,6 +318,9 @@ V_IDENTIFIER [*?.$_a-zA-Z\[\]\-\!\^\\]([*?.$_a-zA-Z0-9\[\]\-\!\^\\]|::)*
 <WILD>"BYTE"				{ RTOKEN(BYTE); }
 <WILD>"ASCII"				{ RTOKEN(ASCII); }
 <WILD>"ASCIZ"				{ RTOKEN(ASCIZ); }
+<WILD>"ALIGN_SECTOR"			{ RTOKEN(ALIGN_SECTOR); }
+<SCRIPT>"BANK"				{ RTOKEN(BANK); }
+<SCRIPT>"SECTOR"			{ RTOKEN(SECTOR); }
 <SCRIPT>"NOFLOAT"			{ RTOKEN(NOFLOAT); }
 <SCRIPT,EXPRESSION>"NOCROSSREFS"	{ RTOKEN(NOCROSSREFS); }
 <SCRIPT,EXPRESSION>"NOCROSSREFS_TO"	{ RTOKEN(NOCROSSREFS_TO); }
-- 
2.34.1

[PATCH v1 4/7] SECTOR: ldgram.y

[binutils]

From: Ulf Samuelsson <ulf@emagii.com>

Parse BANK definition with SECTOR lists.
Defines the begin, end and size of a flash sector.

Parse ALIGN_SECTOR command that will
move the location counter just past the current
flash sector.

Signed-off-by: Ulf Samuelsson <ulf@emagii.com>
---
 ld/ldgram.y | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/ld/ldgram.y b/ld/ldgram.y
index 93aff6eeb76..d6a7f32f098 100644
--- a/ld/ldgram.y
+++ b/ld/ldgram.y
@@ -145,6 +145,7 @@ static int error_index;
 %token NOLOAD DSECT COPY INFO OVERLAY
 %token READONLY
 %token TYPE
+%token BANK SECTOR ALIGN_SECTOR
 %token DEFINED TARGET_K SEARCH_DIR MAP ENTRY
 %token <integer> NEXT
 %token SIZEOF ALIGNOF ADDR LOADADDR MAX_K MIN_K
@@ -309,6 +310,9 @@ ifile_list:
 
 ifile_p1:
 		memory
+	|	BANK '(' NAME ')'
+		{ lang_add_bank($3); }
+		'{' sector_list '}'
 	|	sections
 	|	phdrs
 	|	startup
@@ -367,6 +371,17 @@ ifile_p1:
 		{ lang_ld_feature ($3); }
 	;
 
+sector_list:
+	sector_list sector
+	|
+	;
+
+sector:
+		SECTOR '(' NAME ')' {
+			lang_add_sector($3);
+		} ';'
+	;
+
 input_list:
 		{ ldlex_inputlist(); }
 		input_list1
@@ -714,6 +729,10 @@ statement:
 		{
 		  lang_add_timestamp ();
 		}
+	| ALIGN_SECTOR
+		{
+		  lang_align_sector ();
+		}
 	| ASSERT_K
 		{ ldlex_expression (); }
 	  '(' exp ',' NAME ')' separator
-- 
2.34.1

[PATCH v1 5/7] SECTOR: language additions

[binutils]

From: Ulf Samuelsson <ulf@emagii.com>

Signed-off-by: Ulf Samuelsson <ulf@emagii.com>
---
 ld/lddigest.h  |   7 ++
 ld/ldsectors.c | 309 +++++++++++++++++++++++++++++++++++++++++++++++++
 ld/sysdep.h    |   2 +
 3 files changed, 318 insertions(+)
 create mode 100644 ld/ldsectors.c

diff --git a/ld/lddigest.h b/ld/lddigest.h
index 8f2889f1846..95e442006ef 100755
--- a/ld/lddigest.h
+++ b/ld/lddigest.h
@@ -189,4 +189,11 @@ extern void lang_generate_crc
 extern void lang_generate_digest
   (void);
 
+extern void lang_add_bank
+  (const char *name);
+extern void lang_add_sector
+  (const char *size);
+extern void lang_align_sector
+  (void);
+
 #endif /* LDDIGEST_H */
diff --git a/ld/ldsectors.c b/ld/ldsectors.c
new file mode 100644
index 00000000000..c9e1fcb8756
--- /dev/null
+++ b/ld/ldsectors.c
@@ -0,0 +1,309 @@
+/* Linker command language support.
+   Copyright (C) 1991-2023 Ulf Samuelsson <ulf@emagii.com>
+
+   This file is part of the GNU Binutils.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+   MA 02110-1301, USA.  */
+
+#define _GNU_SOURCE
+#include "sysdep.h"
+#include "bfd.h"
+#include "safe-ctype.h"
+#include "obstack.h"
+#include "bfdlink.h"
+#include "ctf-api.h"
+
+#include "ld.h"
+#include "ldmain.h"
+#include "ldexp.h"
+#include "ldlang.h"
+#include <ldgram.h>
+#include "ldlex.h"
+#include "ldmisc.h"
+#include "lddigest.h"
+
+typedef struct
+{
+  const char *name;
+  int count;
+  int location;
+} bank_t;
+
+typedef struct symbol
+{
+  uint64_t value;
+  char *str;
+} symbol_t;
+
+typedef struct sector
+{
+  symbol_t begin;
+  symbol_t end;
+  symbol_t size;
+  const char *name;
+} sector_t;
+
+typedef struct list list_t;
+struct list
+{
+  list_t *next;
+  void *data;
+};
+
+typedef struct
+{
+  list_t *first;
+  list_t *last;
+  char *name;
+} head_t;
+
+bank_t dummy_bank = {.name = "dummy",.count = 0 };
+list_t dummy_bank_l = {.next = NULL,.data = (void *) &dummy_bank };
+head_t banks = { &dummy_bank_l, &dummy_bank_l, "banks" };
+
+sector_t dummy_sector = {
+  .begin = {0, NULL},
+  .end = {0, NULL},
+  .size = {0, NULL},
+  .name = NULL
+};
+list_t dummy_sector_l = {.next = NULL,.data = (void *) &dummy_sector };
+head_t sectors = { &dummy_sector_l, &dummy_sector_l, "sectors" };
+
+#if 0
+static void
+print_symbol (symbol_t * s)
+{
+  if (s != NULL)
+    {
+      printf ("%-32s= 0x%016lx;\n", s->str, s->value);
+    }
+}
+
+static void
+print_sector (sector_t * s)
+{
+  print_symbol (&s->begin);
+  print_symbol (&s->end);
+  print_symbol (&s->size);
+}
+#endif
+
+static list_t *
+list_add_to_end (head_t * h, void *element)
+{
+  list_t *e = (list_t *) malloc (sizeof (list_t));
+  if (e != NULL)
+    {
+      e->next = NULL;
+      e->data = element;
+      if (h->first == NULL)
+	{
+	  h->first = e;
+	}
+      h->last->next = (void *) e;
+      h->last = (void *) e;
+    }
+  return e;
+}
+
+void
+lang_add_bank (const char *name)
+{
+  bank_t *b = malloc (sizeof (bank_t));
+  list_t *l;
+  if (b == NULL)
+    {
+      einfo (_("%F%P: can not allocate memory for memory bank list: %E\n"));
+      return;
+    }
+  b->name = name;
+  b->count = 0;
+  l = list_add_to_end (&banks, (void *) b);
+  if (l == NULL)
+    {
+      einfo (_("%F%P: can not allocate memory for memory bank: %E\n"));
+    }
+}
+
+void
+lang_add_sector (const char *size)
+{
+  char *p, c;
+  bank_t *b;
+  uint64_t val = strtoull (size, &p, 10);
+  if (errno == ERANGE)
+    return;
+  /* Skip whitespace */
+  while ((c = *p) != 0)
+    {
+      if (c <= ' ')
+	{
+	  p++;
+	}
+      else
+	{
+	  break;
+	}
+    }
+
+  /* Translate to upper case */
+  {
+    char *tmpp = p;
+
+    while ((c = *tmpp) != 0)
+      {
+	if ((c >= 'a') && (c <= 'z'))
+	  {
+	    c = c - 'a' + 'A';
+	    *tmpp = c;
+	  }
+	tmpp++;
+      }
+  }
+
+  /* check for multipliers */
+  if (!strcmp (p, "BYTES"))
+    {
+      val *= 1;
+    }
+  else if (!strcmp (p, "KB"))
+    {
+      val *= 1024;
+    }
+  else if (!strcmp (p, "MB"))
+    {
+      val *= 1024 * 1024;
+    }
+
+  /* Create a sector record - unless dummy sector */
+  b = banks.last->data;
+  if (strncmp (b->name, "dummy", 5))
+    {
+      size_t len;
+      list_t *l;
+      sector_t *s = malloc (sizeof (sector_t));
+      if (s == NULL)
+	{
+	  einfo (_("%F%P: can not allocate memory for sector list: %E\n"));
+	  return;
+	}
+
+      s->begin.value = b->location;
+      len = asprintf (&s->begin.str, "%s#%02d#begin", b->name, b->count);
+      s->end.value = b->location + val;
+      len += asprintf (&s->end.str, "%s#%02d#end", b->name, b->count);
+      s->size.value = val;
+      len += asprintf (&s->size.str, "%s#%02d#size", b->name, b->count);
+      if (len <= 1)
+	{
+	};			/* 'Use' "len" to avoid warnings */
+      l = list_add_to_end (&sectors, (void *) s);
+      if (l == NULL)
+	{
+	  einfo (_("%F%P: can not allocate memory for sector: %E\n"));
+	}
+    }
+  b->location += val;
+  b->count++;
+}
+
+static etree_type *
+dot (void)
+{
+  return exp_nameop (NAME, ".");
+}
+
+static etree_type *
+begin_s (sector_t * s)
+{
+  return exp_nameop (NAME, s->begin.str);
+}
+
+static etree_type *
+end_s (sector_t * s)
+{
+  return exp_nameop (NAME, s->end.str);
+}
+
+static etree_type *
+size_s (sector_t * s)
+{
+  return exp_nameop (NAME, s->size.str);
+}
+
+static void
+define_symbol (symbol_t * symbol)
+{
+  etree_type *value = exp_bigintop ((bfd_vma) symbol->value, NULL);
+  lang_add_assignment (exp_assign (symbol->str, value, true));
+}
+
+static void
+define_sector (sector_t * s)
+{
+  define_symbol (&s->begin);
+  define_symbol (&s->end);
+  define_symbol (&s->size);
+}
+
+static bool
+define_sectors (void)
+{
+  sector_t *s;
+  bool present = false;
+  for (list_t * sl = sectors.first->next; sl != NULL; sl = sl->next)
+    {
+      s = (sector_t *) sl->data;
+      define_sector (s);
+      present = true;
+    }
+  return present;
+}
+
+static void
+cond_align_to_sector (sector_t * s)
+{
+  /*
+   * Align, if location is within the sector
+   * . = ( (. <= begin) && (. <= end) ) ? ALIGN(size) : . ;
+   */
+  etree_type *lower = exp_binop (GE, dot (), begin_s (s));
+  etree_type *upper = exp_binop (LE, dot (), end_s (s));
+  etree_type *in_range = exp_binop (ANDAND, lower, upper);
+  etree_type *align = exp_unop (ALIGN_K, size_s (s));
+  etree_type *expr = exp_trinop ('?', in_range, align, dot ());
+  lang_add_assignment (exp_assign (".", expr, false));
+}
+
+void
+lang_align_sector (void)
+{
+  static bool defined = false;
+  if (!defined)
+    {
+      defined = define_sectors ();
+    }
+  if (!defined)
+    {
+      einfo (_("%F%P: 'ALIGN_SECTOR' needs to be preceeded by a"
+	       " 'BANK' command\n"));
+    }
+  for (list_t * sl = sectors.first->next; sl != NULL; sl = sl->next)
+    {
+      cond_align_to_sector ((sector_t *) sl->data);
+    }
+}
diff --git a/ld/sysdep.h b/ld/sysdep.h
index 3601a59a6ac..7d25e83ef53 100644
--- a/ld/sysdep.h
+++ b/ld/sysdep.h
@@ -41,6 +41,8 @@
 #include <unistd.h>
 #endif
 
+#include <errno.h>
+
 #ifdef HAVE_REALPATH
 # define REALPATH(a,b) realpath (a, b)
 #else
-- 
2.34.1

[PATCH v1 6/7] SECTOR: add testsuite

[binutils]

From: Ulf Samuelsson <ulf@emagii.com>

Add three tests for the ALIGN_SECTOR command.

1. Normal case
2. ALIGN_SECTOR used without a BANK command
3. Adding "dummy" BANK command to shift addresses.

Signed-off-by: Ulf Samuelsson <ulf@emagii.com>
---
 ld/testsuite/ld-scripts/align.exp            |  3 +
 ld/testsuite/ld-scripts/align_dummy_sector.d | 38 ++++++++++++
 ld/testsuite/ld-scripts/align_dummy_sector.s |  4 ++
 ld/testsuite/ld-scripts/align_dummy_sector.t | 61 ++++++++++++++++++++
 ld/testsuite/ld-scripts/align_sector.d       | 38 ++++++++++++
 ld/testsuite/ld-scripts/align_sector.s       |  4 ++
 ld/testsuite/ld-scripts/align_sector.t       | 61 ++++++++++++++++++++
 ld/testsuite/ld-scripts/align_sector_fail.d  |  8 +++
 ld/testsuite/ld-scripts/align_sector_fail.s  |  4 ++
 ld/testsuite/ld-scripts/align_sector_fail.t  | 38 ++++++++++++
 10 files changed, 259 insertions(+)
 create mode 100644 ld/testsuite/ld-scripts/align_dummy_sector.d
 create mode 100644 ld/testsuite/ld-scripts/align_dummy_sector.s
 create mode 100644 ld/testsuite/ld-scripts/align_dummy_sector.t
 create mode 100644 ld/testsuite/ld-scripts/align_sector.d
 create mode 100644 ld/testsuite/ld-scripts/align_sector.s
 create mode 100644 ld/testsuite/ld-scripts/align_sector.t
 create mode 100644 ld/testsuite/ld-scripts/align_sector_fail.d
 create mode 100644 ld/testsuite/ld-scripts/align_sector_fail.s
 create mode 100644 ld/testsuite/ld-scripts/align_sector_fail.t

diff --git a/ld/testsuite/ld-scripts/align.exp b/ld/testsuite/ld-scripts/align.exp
index 382a711b8cf..d9f39e42c68 100644
--- a/ld/testsuite/ld-scripts/align.exp
+++ b/ld/testsuite/ld-scripts/align.exp
@@ -46,6 +46,9 @@ if ![is_aout_format] {
     run_dump_test align3
     run_dump_test align4
     run_dump_test align5
+    run_dump_test align_sector
+    run_dump_test align_dummy_sector
+    run_dump_test align_sector_fail
 }
 run_dump_test align2c
 set LDFLAGS "$saved_LDFLAGS"
diff --git a/ld/testsuite/ld-scripts/align_dummy_sector.d b/ld/testsuite/ld-scripts/align_dummy_sector.d
new file mode 100644
index 00000000000..2198ef9b285
--- /dev/null
+++ b/ld/testsuite/ld-scripts/align_dummy_sector.d
@@ -0,0 +1,38 @@
+# source: align_dummy_sector.s
+# ld: -T align_dummy_sector.t
+# target: [is_elf_format] [is_coff_format]
+# notarget: [is_aout_format]
+# skip: tic4x-*-* tic54x-*-*
+# skip: ns32k-pc532-macho, pdp11-dec-aout, powerpc-ibm-aix5.2.0
+# skip: rs6000-aix4.3.3, alpha-linuxecoff
+# objdump: -s -j .text
+
+.*:     file format .*
+
+Contents of section .text:
+ 10000 73656374 6f723000 00000000 ffffffff  sector0.........
+ 10010 ffffffff ffffffff ffffffff ffffffff  ................
+#...
+ 103f0 ffffffff ffffffff ffffffff ffffffff  ................
+ 10400 73656374 6f723100 ffffffff ffffffff  sector1.........
+ 10410 ffffffff ffffffff ffffffff ffffffff  ................
+#...
+ 107f0 ffffffff ffffffff ffffffff ffffffff  ................
+ 10800 73656374 6f723200 ffffffff ffffffff  sector2.........
+ 10810 ffffffff ffffffff ffffffff ffffffff  ................
+#...
+ 10bf0 ffffffff ffffffff ffffffff ffffffff  ................
+ 10c00 73656374 6f723300 ffffffff ffffffff  sector3.........
+ 10c10 ffffffff ffffffff ffffffff ffffffff  ................
+#...
+ 10ff0 ffffffff ffffffff ffffffff ffffffff  ................
+ 11000 73656374 6f723400 ffffffff ffffffff  sector4.........
+ 11010 ffffffff ffffffff ffffffff ffffffff  ................
+#...
+ 117f0 ffffffff ffffffff ffffffff ffffffff  ................
+ 11800 73656374 6f723500 ffffffff ffffffff  sector5.........
+ 11810 ffffffff ffffffff ffffffff ffffffff  ................
+#...
+ 11ff0 ffffffff ffffffff ffffffff ffffffff  ................
+ 12000 73656374 6f723600 ffffffff ffffffff  sector6.........
+#pass
diff --git a/ld/testsuite/ld-scripts/align_dummy_sector.s b/ld/testsuite/ld-scripts/align_dummy_sector.s
new file mode 100644
index 00000000000..4e56d4a8e53
--- /dev/null
+++ b/ld/testsuite/ld-scripts/align_dummy_sector.s
@@ -0,0 +1,4 @@
+	.text
+	.long 0
+	.data
+	.long 0x12345678
diff --git a/ld/testsuite/ld-scripts/align_dummy_sector.t b/ld/testsuite/ld-scripts/align_dummy_sector.t
new file mode 100644
index 00000000000..e8d8eaecc94
--- /dev/null
+++ b/ld/testsuite/ld-scripts/align_dummy_sector.t
@@ -0,0 +1,61 @@
+MEMORY {
+  rom : ORIGIN = 0x10000, LENGTH = 0x4000
+  ram : ORIGIN = 0x14000, LENGTH = 0x1000
+}
+
+BANK ( dummy0 ) {
+  SECTOR ("64kB");
+}
+
+BANK ( bank0 ) {
+  SECTOR ("1 KB");
+  SECTOR ("1 KB");
+  SECTOR ("1 KB");
+  SECTOR ("1 KB");
+  SECTOR ("2 KB");
+  SECTOR ("2 KB");
+}
+
+SECTIONS
+{
+  .text : {
+	. = ALIGN(0x8000);
+	FILL(0xFF)
+	sector0 = .;
+	ASCIZ "sector0"
+	*(.text .pr)
+	ALIGN_SECTOR
+	sector1 = .;
+	ASCIZ "sector1"
+	ALIGN_SECTOR
+	sector2 = .;
+	ASCIZ "sector2"
+	ALIGN_SECTOR
+	sector3 = .;
+	ASCIZ "sector3"
+	ALIGN_SECTOR
+	sector4 = .;
+	ASCIZ "sector4"
+	ALIGN_SECTOR
+	sector5 = .;
+	ASCIZ "sector5"
+	ALIGN_SECTOR
+	sector6 = .;
+	ASCIZ "sector6"
+	. = ALIGN(0x10);
+  } >  rom
+  .data : AT (0x400000) { *(.data) } >ram /* NO default AT>rom */
+  . = ALIGN(0x20);
+  .bss : { *(.bss) } >ram /* NO default AT>rom */
+
+  /*
+  ASSERT (sector0 == 0x0000, "Bad Address: sector0")
+  ASSERT (sector1 == 0x0400, "Bad Address: sector1")
+  ASSERT (sector2 == 0x0800, "Bad Address: sector2")
+  ASSERT (sector3 == 0x0C00, "Bad Address: sector3")
+  ASSERT (sector4 == 0x1000, "Bad Address: sector4")
+  ASSERT (sector5 == 0x1800, "Bad Address: sector5")
+  ASSERT (sector6 == 0x2000, "Bad Address: sector6")
+  */
+  /DISCARD/ : { *(*) }
+}
diff --git a/ld/testsuite/ld-scripts/align_sector.d b/ld/testsuite/ld-scripts/align_sector.d
new file mode 100644
index 00000000000..db95221ab2a
--- /dev/null
+++ b/ld/testsuite/ld-scripts/align_sector.d
@@ -0,0 +1,38 @@
+# source: align_sector.s
+# ld: -T align_sector.t
+# target: [is_elf_format] [is_coff_format]
+# notarget: [is_aout_format]
+# skip: tic4x-*-* tic54x-*-*
+# skip: ns32k-pc532-macho, pdp11-dec-aout, powerpc-ibm-aix5.2.0
+# skip: rs6000-aix4.3.3, alpha-linuxecoff
+# objdump: -s -j .text
+
+.*:     file format .*
+
+Contents of section .text:
+ 0000 73656374 6f723000 00000000 ffffffff  sector0.........
+ 0010 ffffffff ffffffff ffffffff ffffffff  ................
+#...
+ 03f0 ffffffff ffffffff ffffffff ffffffff  ................
+ 0400 73656374 6f723100 ffffffff ffffffff  sector1.........
+ 0410 ffffffff ffffffff ffffffff ffffffff  ................
+#...
+ 07f0 ffffffff ffffffff ffffffff ffffffff  ................
+ 0800 73656374 6f723200 ffffffff ffffffff  sector2.........
+ 0810 ffffffff ffffffff ffffffff ffffffff  ................
+#...
+ 0bf0 ffffffff ffffffff ffffffff ffffffff  ................
+ 0c00 73656374 6f723300 ffffffff ffffffff  sector3.........
+ 0c10 ffffffff ffffffff ffffffff ffffffff  ................
+#...
+ 0ff0 ffffffff ffffffff ffffffff ffffffff  ................
+ 1000 73656374 6f723400 ffffffff ffffffff  sector4.........
+ 1010 ffffffff ffffffff ffffffff ffffffff  ................
+#...
+ 17f0 ffffffff ffffffff ffffffff ffffffff  ................
+ 1800 73656374 6f723500 ffffffff ffffffff  sector5.........
+ 1810 ffffffff ffffffff ffffffff ffffffff  ................
+#...
+ 1ff0 ffffffff ffffffff ffffffff ffffffff  ................
+ 2000 73656374 6f723600 ffffffff ffffffff  sector6.........
+#pass
diff --git a/ld/testsuite/ld-scripts/align_sector.s b/ld/testsuite/ld-scripts/align_sector.s
new file mode 100644
index 00000000000..4e56d4a8e53
--- /dev/null
+++ b/ld/testsuite/ld-scripts/align_sector.s
@@ -0,0 +1,4 @@
+	.text
+	.long 0
+	.data
+	.long 0x12345678
diff --git a/ld/testsuite/ld-scripts/align_sector.t b/ld/testsuite/ld-scripts/align_sector.t
new file mode 100644
index 00000000000..83093e048da
--- /dev/null
+++ b/ld/testsuite/ld-scripts/align_sector.t
@@ -0,0 +1,61 @@
+MEMORY {
+  rom : ORIGIN = 0x0000, LENGTH = 0x4000
+  ram : ORIGIN = 0x4000, LENGTH = 0x1000
+}
+
+BANK ( dummy0 ) {
+  SECTOR ("32kB");
+}
+
+BANK ( bank0 ) {
+  SECTOR ("1 KB");
+  SECTOR ("1 KB");
+  SECTOR ("1 KB");
+  SECTOR ("1 KB");
+  SECTOR ("2 KB");
+  SECTOR ("2 KB");
+}
+
+SECTIONS
+{
+  .text : {
+	. = ALIGN(0x8000);
+	FILL(0xFF)
+	sector0 = .;
+	ASCIZ "sector0"
+	*(.text .pr)
+	ALIGN_SECTOR
+	sector1 = .;
+	ASCIZ "sector1"
+	ALIGN_SECTOR
+	sector2 = .;
+	ASCIZ "sector2"
+	ALIGN_SECTOR
+	sector3 = .;
+	ASCIZ "sector3"
+	ALIGN_SECTOR
+	sector4 = .;
+	ASCIZ "sector4"
+	ALIGN_SECTOR
+	sector5 = .;
+	ASCIZ "sector5"
+	ALIGN_SECTOR
+	sector6 = .;
+	ASCIZ "sector6"
+	. = ALIGN(0x10);
+  } >  rom
+  .data : AT (0x400000) { *(.data) } >ram /* NO default AT>rom */
+  . = ALIGN(0x20);
+  .bss : { *(.bss) } >ram /* NO default AT>rom */
+
+  /*
+  ASSERT (sector0 == 0x0000, "Bad Address: sector0")
+  ASSERT (sector1 == 0x0400, "Bad Address: sector1")
+  ASSERT (sector2 == 0x0800, "Bad Address: sector2")
+  ASSERT (sector3 == 0x0C00, "Bad Address: sector3")
+  ASSERT (sector4 == 0x1000, "Bad Address: sector4")
+  ASSERT (sector5 == 0x1800, "Bad Address: sector5")
+  ASSERT (sector6 == 0x2000, "Bad Address: sector6")
+  */
+  /DISCARD/ : { *(*) }
+}
diff --git a/ld/testsuite/ld-scripts/align_sector_fail.d b/ld/testsuite/ld-scripts/align_sector_fail.d
new file mode 100644
index 00000000000..228009804a9
--- /dev/null
+++ b/ld/testsuite/ld-scripts/align_sector_fail.d
@@ -0,0 +1,8 @@
+# source: align_sector_fail.s
+# ld: -T align_sector_fail.t
+# error: .*: 'ALIGN_SECTOR' needs to be preceeded by a 'BANK' command.*
+# target: [is_elf_format] [is_coff_format]
+# notarget: [is_aout_format]
+# skip: tic4x-*-* tic54x-*-*
+# skip: ns32k-pc532-macho, pdp11-dec-aout, powerpc-ibm-aix5.2.0
+# skip: rs6000-aix4.3.3, alpha-linuxecoff
diff --git a/ld/testsuite/ld-scripts/align_sector_fail.s b/ld/testsuite/ld-scripts/align_sector_fail.s
new file mode 100644
index 00000000000..4e56d4a8e53
--- /dev/null
+++ b/ld/testsuite/ld-scripts/align_sector_fail.s
@@ -0,0 +1,4 @@
+	.text
+	.long 0
+	.data
+	.long 0x12345678
diff --git a/ld/testsuite/ld-scripts/align_sector_fail.t b/ld/testsuite/ld-scripts/align_sector_fail.t
new file mode 100644
index 00000000000..1e1dc231488
--- /dev/null
+++ b/ld/testsuite/ld-scripts/align_sector_fail.t
@@ -0,0 +1,38 @@
+MEMORY {
+  rom : ORIGIN = 0x0000, LENGTH = 0x4000
+  ram : ORIGIN = 0x4000, LENGTH = 0x1000
+}
+
+SECTIONS
+{
+  .text : {
+	FILL(0xFF)
+	sector0 = .;
+	ASCIZ "sector0"
+	*(.text .pr)
+	ALIGN_SECTOR
+	sector1 = .;
+	ASCIZ "sector1"
+	ALIGN_SECTOR
+	sector2 = .;
+	ASCIZ "sector2"
+	ALIGN_SECTOR
+	sector3 = .;
+	ASCIZ "sector3"
+	ALIGN_SECTOR
+	sector4 = .;
+	ASCIZ "sector4"
+	ALIGN_SECTOR
+	sector5 = .;
+	ASCIZ "sector5"
+	ALIGN_SECTOR
+	sector6 = .;
+	ASCIZ "sector6"
+	. = ALIGN(0x10);
+  } >  rom
+  .data : AT (0x4000) { *(.data) } >ram /* NO default AT>rom */
+  . = ALIGN(0x20);
+  .bss : { *(.bss) } >ram /* NO default AT>rom */
+
+  /DISCARD/ : { *(*) }
+}
-- 
2.34.1

[PATCH v1 7/7] SECTOR: Makefile.*

[binutils]

From: Ulf Samuelsson <ulf@emagii.com>

Makefile.in has been manually edited due to tool problems.

Signed-off-by: Ulf Samuelsson <ulf@emagii.com>
---
 ld/Makefile.am | 6 +++---
 ld/Makefile.in | 9 +++++----
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/ld/Makefile.am b/ld/Makefile.am
index 00118f8da13..5927d125a46 100644
--- a/ld/Makefile.am
+++ b/ld/Makefile.am
@@ -479,7 +479,7 @@ CFILES = ldctor.c ldemul.c ldexp.c ldfile.c ldlang.c \
 	ldmain.c ldmisc.c ldver.c ldwrite.c lexsup.c \
 	mri.c ldcref.c pe-dll.c pep-dll.c ldlex-wrapper.c \
 	plugin.c ldbuildid.c ldelf.c ldelfgen.c \
-	lddigest.c lddigest_tab.c ldcrc32.c ldcrc64.c ldreflect.c \
+	lddigest.c lddigest_tab.c ldcrc32.c ldcrc64.c ldreflect.c ldsectors.c \
 	pdb.c
 
 HFILES = ld.h ldctor.h ldemul.h ldexp.h ldfile.h \
@@ -497,7 +497,7 @@ GENERATED_HFILES = ldgram.h ldemul-list.h deffilep.h
 BUILT_SOURCES = $(GENERATED_HFILES)
 
 OFILES = ldgram.@OBJEXT@ ldlex-wrapper.@OBJEXT@ lexsup.@OBJEXT@ ldlang.@OBJEXT@ \
-	lddigest.@OBJEXT@ lddigest_tab.@OBJEXT@ \
+	lddigest.@OBJEXT@ lddigest_tab.@OBJEXT@ ldsectors.@OBJEXT@ \
 	ldcrc32.@OBJEXT@ ldcrc64.@OBJEXT@ ldreflect.@OBJEXT@ \
 	mri.@OBJEXT@ ldctor.@OBJEXT@ ldmain.@OBJEXT@ plugin.@OBJEXT@ \
 	ldwrite.@OBJEXT@ ldexp.@OBJEXT@  ldemul.@OBJEXT@ ldver.@OBJEXT@ ldmisc.@OBJEXT@ \
@@ -966,7 +966,7 @@ EXTRA_ld_new_SOURCES = deffilep.y ldlex.l
 EXTRA_ld_new_SOURCES += ldelf.c ldelfgen.c pdb.c pep-dll.c pe-dll.c
 
 ld_new_SOURCES = ldgram.y ldlex-wrapper.c lexsup.c ldlang.c mri.c ldctor.c ldmain.c \
-	lddigest.c lddigest_tab.c ldcrc32.c ldcrc64.c ldreflect.c \
+	lddigest.c lddigest_tab.c ldcrc32.c ldcrc64.c ldreflect.c ldsectors.c \
 	ldwrite.c ldexp.c ldemul.c ldver.c ldmisc.c ldfile.c ldcref.c plugin.c \
 	ldbuildid.c
 ld_new_DEPENDENCIES = $(EMULATION_OFILES) $(EMUL_EXTRA_OFILES) \
diff --git a/ld/Makefile.in b/ld/Makefile.in
index 7bf51bffa54..1da95450298 100644
--- a/ld/Makefile.in
+++ b/ld/Makefile.in
@@ -210,7 +210,7 @@ PROGRAMS = $(bin_PROGRAMS)
 am_ld_new_OBJECTS = ldgram.$(OBJEXT) ldlex-wrapper.$(OBJEXT) \
 	lexsup.$(OBJEXT) ldlang.$(OBJEXT) mri.$(OBJEXT) \
 	ldctor.$(OBJEXT) ldmain.$(OBJEXT) ldwrite.$(OBJEXT) \
-	lddigest.$(OBJEXT) lddigest_tab.$(OBJEXT) \
+	lddigest.$(OBJEXT) lddigest_tab.$(OBJEXT) ldsectors.$(OBJEXT) \
 	ldcrc32.$(OBJEXT) ldcrc64.$(OBJEXT) ldreflect.$(OBJEXT) \
 	ldexp.$(OBJEXT) ldemul.$(OBJEXT) ldver.$(OBJEXT) \
 	ldmisc.$(OBJEXT) ldfile.$(OBJEXT) ldcref.$(OBJEXT) \
@@ -981,7 +981,7 @@ CFILES = ldctor.c ldemul.c ldexp.c ldfile.c ldlang.c \
 	ldmain.c ldmisc.c ldver.c ldwrite.c lexsup.c \
 	mri.c ldcref.c pe-dll.c pep-dll.c ldlex-wrapper.c \
 	plugin.c ldbuildid.c ldelf.c ldelfgen.c \
-	lddigest.c lddigest_tab.c ldcrc32.c ldcrc64.c ldreflect.c \
+	lddigest.c lddigest_tab.c ldcrc32.c ldcrc64.c ldreflect.c ldsectors.c \
 	pdb.c
 
 HFILES = ld.h ldctor.h ldemul.h ldexp.h ldfile.h \
@@ -998,7 +998,7 @@ GENERATED_HFILES = ldgram.h ldemul-list.h deffilep.h
 # tracking will not cause them to be built beforehand.
 BUILT_SOURCES = $(GENERATED_HFILES)
 OFILES = ldgram.@OBJEXT@ ldlex-wrapper.@OBJEXT@ lexsup.@OBJEXT@ ldlang.@OBJEXT@ \
-	lddigest.@OBJEXT@ lddigest_tab.@OBJEXT@ \
+	lddigest.@OBJEXT@ lddigest_tab.@OBJEXT@ ldsectors.@OBJEXT@ \
 	ldcrc32.@OBJEXT@ ldcrc64.@OBJEXT@ ldreflect.@OBJEXT@ \
 	mri.@OBJEXT@ ldctor.@OBJEXT@ ldmain.@OBJEXT@ plugin.@OBJEXT@ \
 	ldwrite.@OBJEXT@ ldexp.@OBJEXT@  ldemul.@OBJEXT@ ldver.@OBJEXT@ ldmisc.@OBJEXT@ \
@@ -1021,7 +1021,7 @@ EXTRA_ld_new_SOURCES = deffilep.y ldlex.l ldelf.c ldelfgen.c pdb.c \
 	pep-dll.c pe-dll.c $(ALL_EMULATION_SOURCES) \
 	$(ALL_64_EMULATION_SOURCES)
 ld_new_SOURCES = ldgram.y ldlex-wrapper.c lexsup.c ldlang.c mri.c ldctor.c ldmain.c \
-	lddigest.c lddigest_tab.c ldcrc32.c ldcrc64.c ldreflect.c \
+	lddigest.c lddigest_tab.c ldcrc32.c ldcrc64.c ldreflect.c ldsectors.c \
 	ldwrite.c ldexp.c ldemul.c ldver.c ldmisc.c ldfile.c ldcref.c plugin.c \
 	ldbuildid.c
 
@@ -1574,6 +1574,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ldctor.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/lddigest.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/lddigest_tab.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ldsectors.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ldelf.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ldelfgen.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ldemul.Po@am__quote@
-- 
2.34.1

Re: [PATCH v1 0/7 SECTOR: Support aligning to flash sector boundary

[Alan Modra]

On Fri, Mar 10, 2023 at 01:08:10AM +0100, Ulf Samuelsson via Binutils wrote:
> Patchset 1
> 
> Introduce the BANK command which allows
> the user to define flash sector boundaries.

Ulf, there is a serious problem with your binutils contributions:
lddigest.c: "Copyright (C) 1991-2023 Ulf Samuelsson <ulf@emagii.com>".

According to
https://www.gnu.org/prep/maintain/maintain.html#Legal-Matters
I as a maintainer of a GNU package need to ensure certain steps are
taken with legally significant contributions.  I can't see Ulf
Samuelsson listed on the current GNU copyright list, and I don't
recall seeing an email from the FSF copyright clerk saying that Ulf
had assigned or disclaimed copyright.

So unless I've missed something, all of the crc/digest/ascii code
should be reverted from binutils until copyright papers are signed.
Nick agrees.  I'm sorry we didn't sort this out earlier, but once that
has been done they can be reapplied.

You can start the copyright process by looking at doc/Copyright/ in
current gnulib, and filling out the most appropriate, probably
request-assign.future.  The FSF will then send you their copyright
assignment forms.  I'm not certain about files like ldcrc64.c with the
MIT license.  They likely are OK as is, since we have other examples
with similar license in binutils under gprof/ and libiberty/.  Please
discuss that detail with the FSF.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH v1 0/7 SECTOR: Support aligning to flash sector boundary

[Michael Matz]

Hello,

On Fri, 10 Mar 2023, Ulf Samuelsson via Binutils wrote:

> This can be used later to align a sector to the flash boundary using the 
> 
> ALIGN_SECTOR command in an output sector.

Please consider implementing new features in terms of existing ones.  For 
instance there is already an 'ALIGN' expression, that either takes dot or 
an arbitrary expression to align, to a certain given value.  Your new 
feature is basically just a special value for such alignment, so it makes 
sense to specify _that_ instead of a new top-level expression.  So, 
consider accepting something like:

   ALIGN(TOSECTOR)
   ALIGN(expr, TOSECTOR)

so that one would write

   . = ALIGN(TOSECTOR)

as usual with alignment of dot.  The 'TOSECTOR' token would be the new 
thing and make it do what you wanted with ALIGN_SECTOR.


Ciao,
Michael.

Re: [PATCH v1 0/7 SECTOR: Support aligning to flash sector boundary

[Ulf Samuelsson]

On 2023-03-10 15:13, Michael Matz wrote:
> Hello,
>
> On Fri, 10 Mar 2023, Ulf Samuelsson via Binutils wrote:
>
>> This can be used later to align a sector to the flash boundary using the
>>
>> ALIGN_SECTOR command in an output sector.
> Please consider implementing new features in terms of existing ones.  For
> instance there is already an 'ALIGN' expression, that either takes dot or
> an arbitrary expression to align, to a certain given value.  Your new
> feature is basically just a special value for such alignment, so it makes
> sense to specify _that_ instead of a new top-level expression.  So,
> consider accepting something like:
>
>     ALIGN(TOSECTOR)
>     ALIGN(expr, TOSECTOR)
>
> so that one would write
>
>     . = ALIGN(TOSECTOR)

I considered it, but came to the conclusion that it opens a bag of worms.

To fit this into the grammar, TOSECTOR (or maybe just SECTOR)
needs to be a valid expression, returning the size of the flash sector
where the location counter is.

Since expressions are available everywhere, you run into situations
where it really does not make sense for "SECTOR" to have a value.

It only makes sense when the linker is processing the output section,
and even there, it hardly makes sense to use it except as a way to
finish the output section.

The way it is implemented, it is really more of a subroutine than a 
function.
For each declared sector, there are four assignment statements.
One of them is moderately complex.
It is not uncommon for larger microcontrollers to have 32 or more flash 
sectors.
I do not rule out that microcontrollers will have 64 or even 128 flash 
sectors.

To create an function which collapses 100+ statements into a single 
expression
may stress the expression evaluator in unpredictable ways.

Have the linker been tested with 100-200 lines+ expressions?

Another aspect is what part of the code is affected.
Right now, everything except flex/bison code is inside a new file 
(ldsectors.c)

The alternative that I could consider is to add it in the Output Section 
Description
There you can specify ALIGN_WITH_INPUT which has a similar syntax as 
ALIGN_SECTOR
so alignment is specified either using an expression or a special 
keyword already today.

It may be possible to have ALIGN '(' SECTOR ')' as a statement (not an 
expression)

Best Regards
Ulf Samuelsson


>
> as usual with alignment of dot.  The 'TOSECTOR' token would be the new
> thing and make it do what you wanted with ALIGN_SECTOR.
>
>
> Ciao,
> Michael.

Re: [PATCH v1 0/7 SECTOR: Support aligning to flash sector boundary

[Michael Matz]

Hello.

On Fri, 10 Mar 2023, Ulf Samuelsson wrote:

> >     . = ALIGN(TOSECTOR)
> 
> I considered it, but came to the conclusion that it opens a bag of worms.
> 
> To fit this into the grammar, TOSECTOR (or maybe just SECTOR)
> needs to be a valid expression, returning the size of the flash sector
> where the location counter is.
> 
> Since expressions are available everywhere, you run into situations
> where it really does not make sense for "SECTOR" to have a value.

So, make SECTOR an expression taking an address as well.  Call it 
SECTORSIZE and make it return the sector size for the given argument (an 
address, so that 'dot' can be given in output sections), or 1 if no sector 
is associated with the argument.  Then your alignment expression will 
become

  . = ALIGN(SECTORSIZE(.))

and SECTORSIZE now _can_ make sense in other than output sections (when 
you for instance give it, say, a symbolname defined elsewhere in an 
utput section).

> To create an function which collapses 100+ statements into a single 
> expression may stress the expression evaluator in unpredictable ways.
> 
> Have the linker been tested with 100-200 lines+ expressions?

Yes.  Such thoughts shouldn't prevent an orthogonal implementation of 
features anyway.  You can also open-code the implementation of SECTORSIZE 
with a helper routine, not resorting to linkerscript expressions at all, 
if you worry about performance of that.

> The alternative that I could consider is to add it in the Output Section 
> Description There you can specify ALIGN_WITH_INPUT which has a similar 
> syntax as ALIGN_SECTOR so alignment is specified either using an 
> expression or a special keyword already today.

I think having it available as expression to be used in ALIGN (or other) 
expressions is the better choice.


Ciao,
Michael.

Re: [PATCH v1 0/7 SECTOR: Support aligning to flash sector boundary

[Ulf Samuelsson]

Since the proposal defines the begin, end and size of a sector as symbols,
you can do already today.
. = ALIGN(”bank00#04#size”).
Right now, the symbols are defined late, but that is easily changed.

Best Regards
Ulf Samuelsson

> 10 mars 2023 kl. 18:31 skrev Michael Matz <matz@suse.de>:
> 
> Hello.
> 
> On Fri, 10 Mar 2023, Ulf Samuelsson wrote:
> 
>>>    . = ALIGN(TOSECTOR)
>> 
>> I considered it, but came to the conclusion that it opens a bag of worms.
>> 
>> To fit this into the grammar, TOSECTOR (or maybe just SECTOR)
>> needs to be a valid expression, returning the size of the flash sector
>> where the location counter is.
>> 
>> Since expressions are available everywhere, you run into situations
>> where it really does not make sense for "SECTOR" to have a value.
> 
> So, make SECTOR an expression taking an address as well.  Call it 
> SECTORSIZE and make it return the sector size for the given argument (an 
> address, so that 'dot' can be given in output sections), or 1 if no sector 
> is associated with the argument.  Then your alignment expression will 
> become
> 
>  . = ALIGN(SECTORSIZE(.))
> 
> and SECTORSIZE now _can_ make sense in other than output sections (when 
> you for instance give it, say, a symbolname defined elsewhere in an 
> utput section).
> 
>> To create an function which collapses 100+ statements into a single 
>> expression may stress the expression evaluator in unpredictable ways.
>> 
>> Have the linker been tested with 100-200 lines+ expressions?
> 
> Yes.  Such thoughts shouldn't prevent an orthogonal implementation of 
> features anyway.  You can also open-code the implementation of SECTORSIZE 
> with a helper routine, not resorting to linkerscript expressions at all, 
> if you worry about performance of that.
> 
>> The alternative that I could consider is to add it in the Output Section 
>> Description There you can specify ALIGN_WITH_INPUT which has a similar 
>> syntax as ALIGN_SECTOR so alignment is specified either using an 
>> expression or a special keyword already today.
> 
> I think having it available as expression to be used in ALIGN (or other) 
> expressions is the better choice.
> 
> 
> Ciao,
> Michael.

Re: [PATCH v1 0/7 SECTOR: Support aligning to flash sector boundary

[Michael Matz]

[-- Attachment #1: Type: text/plain, Size: 605 bytes --]

Hello,

On Fri, 10 Mar 2023, Ulf Samuelsson wrote:

> Since the proposal defines the begin, end and size of a sector as symbols,
> you can do already today.
> . = ALIGN(”bank00#04#size”).
> Right now, the symbols are defined late, but that is easily changed.

You argued that one point of you introducing the ALLOCATE_SECTOR directive 
was that it naturally dependend on dot, exactly so that the user does 
_not_ have to deal with the artificial names.

So, a builtin function (however implemented) that actually gets you that 
very size for a given address makes the most sense IMHO.


Ciao,
Michael.

Re: [PATCH v1 0/7 SECTOR: Support aligning to flash sector boundary

[Ulf Samuelsson]

On 2023-03-13 14:12, Michael Matz wrote:
> Hello,
>
> On Fri, 10 Mar 2023, Ulf Samuelsson wrote:
>
>> Since the proposal defines the begin, end and size of a sector as symbols,
>> you can do already today.
>> . = ALIGN(”bank00#04#size”).
>> Right now, the symbols are defined late, but that is easily changed.
> You argued that one point of you introducing the ALLOCATE_SECTOR directive
> was that it naturally dependend on dot, exactly so that the user does
> _not_ have to deal with the artificial names.
>
> So, a builtin function (however implemented) that actually gets you that
> very size for a given address makes the most sense IMHO.

Not really. The proposal is to avoid having to figure out the boundaries 
for each sector.
and to avoid having to maintain files which aligns to the sector.

But I tried implementing

   . = ALIGN(SECTOR);

and that works now.

If "." is within a sector, then it moves "." to beyond the sector,
otherwise it remains as it is.

While

   . = ALIGN(SECTOR("."));

allows you to get the sector for any address, what would be a reasonable 
motivation for having this?

To me, it just adds additional ways of introducing errors by increased 
complexity in the syntax.


Best Regards
Ulf Samuelsson

> Ciao,
> Michael.

Re: [PATCH v1 0/7 SECTOR: Support aligning to flash sector boundary

[Michael Matz]

[-- Attachment #1: Type: text/plain, Size: 2184 bytes --]

Hello,

On Mon, 13 Mar 2023, Ulf Samuelsson wrote:

> > So, a builtin function (however implemented) that actually gets you 
> > that very size for a given address makes the most sense IMHO.
> 
> Not really. The proposal is to avoid having to figure out the boundaries 
> for each sector. and to avoid having to maintain files which aligns to 
> the sector.
> 
> But I tried implementing
> 
>   . = ALIGN(SECTOR);
> 
> and that works now.
> 
> If "." is within a sector, then it moves "." to beyond the sector,
> otherwise it remains as it is.
> 
> While
> 
>   . = ALIGN(SECTOR("."));
> 
> allows you to get the sector for any address, what would be a reasonable
> motivation for having this?

That doesn't matter so much.  The point is that the linker script already 
supports an assortment of expressions, one of them 'ALIGN(EXP,ALIGN)' 
(with EXP being optional and "dot" by default).  This expression can be 
used in arbitrary places right now.  If it makes sense to use it in 
arbitrary places?  Probably not, but who knows the future?  That's why 
syntax extensions to the ldscripts should not necessarily prescribe usage, 
and be orthogonal (that was my ultimate reason for suggesting to have 
something returning the sector-size for a given address).

Basically: if we extend the syntax and can give reasonable and obvious 
meaning to the new constructs and nevertheless put in limits of usage 
right now, then this will eventually bite us in the future: a new usage 
turns up, the old syntax doesn't support it, but needs to be maintained 
for backward compatibility, and hence another new syntax needs to be 
invented that essentially does the same thing as the older syntax that 
turned out to be too limited.

> To me, it just adds additional ways of introducing errors by increased 
> complexity in the syntax.

Linkerscripts are a powerful tool, and many of the ways of how to use them 
incorrectly are because of too many ad-hoc additions at random syntax 
elements over the past decades.  But using "FOOBAR(.)" outside an output 
section, so that "." isn't defined, and hence could be warned about, is 
_not_ a source of much rope to hang yourself.


Ciao,
Michael.

Re: [PATCH v1 0/7 SECTOR: Support aligning to flash sector boundary

[Ulf Samuelsson]

On 2023-03-13 16:54, Michael Matz wrote:
> Hello,
>
> On Mon, 13 Mar 2023, Ulf Samuelsson wrote:
>
>>> So, a builtin function (however implemented) that actually gets you
>>> that very size for a given address makes the most sense IMHO.
>> Not really. The proposal is to avoid having to figure out the boundaries
>> for each sector. and to avoid having to maintain files which aligns to
>> the sector.
>>
>> But I tried implementing
>>
>>    . = ALIGN(SECTOR);
>>
>> and that works now.
>>
>> If "." is within a sector, then it moves "." to beyond the sector,
>> otherwise it remains as it is.
>>
>> While
>>
>>    . = ALIGN(SECTOR("."));
>>
>> allows you to get the sector for any address, what would be a reasonable
>> motivation for having this?
> That doesn't matter so much.  The point is that the linker script already
> supports an assortment of expressions, one of them 'ALIGN(EXP,ALIGN)'
> (with EXP being optional and "dot" by default).  This expression can be
> used in arbitrary places right now.  If it makes sense to use it in
> arbitrary places?  Probably not, but who knows the future?  That's why
> syntax extensions to the ldscripts should not necessarily prescribe usage,
> and be orthogonal (that was my ultimate reason for suggesting to have
> something returning the sector-size for a given address).
SECTOR is a function which returns the size of the sector of the current 
location
and can be used to align an expression to that size.

You can do ALIGN(exp, SECTOR) if you want to. That will align the 
expression to the sector
size of the current location. So there is no problem with orthogonality.

You are asking for a function which does something else.

You can add a function (SECTOR_SIZE '(' exp ')') if you find it useful.

Best Regards
Ulf Samuelsson


>
> Basically: if we extend the syntax and can give reasonable and obvious
> meaning to the new constructs and nevertheless put in limits of usage
> right now, then this will eventually bite us in the future: a new usage
> turns up, the old syntax doesn't support it, but needs to be maintained
> for backward compatibility, and hence another new syntax needs to be
> invented that essentially does the same thing as the older syntax that
> turned out to be too limited.
>
>> To me, it just adds additional ways of introducing errors by increased
>> complexity in the syntax.
> Linkerscripts are a powerful tool, and many of the ways of how to use them
> incorrectly are because of too many ad-hoc additions at random syntax
> elements over the past decades.  But using "FOOBAR(.)" outside an output
> section, so that "." isn't defined, and hence could be warned about, is
> _not_ a source of much rope to hang yourself.
>
>
> Ciao,
> Michael.

Re: [PATCH v1 0/7 SECTOR: Support aligning to flash sector boundary

[Michael Matz]

Hello,

On Mon, 13 Mar 2023, Ulf Samuelsson wrote:

> > That doesn't matter so much.  The point is that the linker script already
> > supports an assortment of expressions, one of them 'ALIGN(EXP,ALIGN)'
> > (with EXP being optional and "dot" by default).  This expression can be
> > used in arbitrary places right now.  If it makes sense to use it in
> > arbitrary places?  Probably not, but who knows the future?  That's why
> > syntax extensions to the ldscripts should not necessarily prescribe usage,
> > and be orthogonal (that was my ultimate reason for suggesting to have
> > something returning the sector-size for a given address).
> 
> SECTOR is a function which returns the size of the sector of the current 
> location and can be used to align an expression to that size.
> 
> You can do ALIGN(exp, SECTOR) if you want to. That will align the 
> expression to the sector size of the current location. So there is no 
> problem with orthogonality.
> 
> You are asking for a function which does something else.
> 
> You can add a function (SECTOR_SIZE '(' exp ')') if you find it useful.

SECTOR without argument is good enough for my taste.  My main objection 
was to the new directive ALIGN_SECTOR; I suggested SECTOR(exp) only in 
response to your worry about a SECTOR expression (without argument, and 
hence an implicit "dot") being used outside output section statements 
without clear meaning.

So, I'm happy :)  Thanks for consideration.


Ciao,
Michael.