mknum.pl: Exclude duplicate entries and include source file name in diagnostics

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14074)

2 files changed, 38 insertions, 21 deletions

diff --git a/util/mknum.pl b/util/mknum.pl
index 45f3381c81..19d9d55108 100644
--- a/util/mknum.pl
+++ b/util/mknum.pl
@@ -73,7 +73,7 @@ foreach my $f (($symhacks_file // (), @ARGV)) {
                 && defined $symhacks_file
                 && $f eq $symhacks_file
                 && $_->{value} =~ /^\w(?:\w|\d)*/) {
-            $ordinals->add_alias($_->{value}, $_->{name}, @{$_->{conds}});
+            $ordinals->add_alias($f, $_->{value}, $_->{name}, @{$_->{conds}});
         } else {
             next if $_->{returntype} =~ /\b(?:ossl_)inline/;
             my $type = {
@@ -81,7 +81,7 @@ foreach my $f (($symhacks_file // (), @ARGV)) {
                 V => 'VARIABLE',
             } -> {$_->{type}};
             if ($type) {
-                $ordinals->add($_->{name}, $type, @{$_->{conds}});
+                $ordinals->add($f, $_->{name}, $type, @{$_->{conds}});
             }
         }
     }
diff --git a/util/perl/OpenSSL/Ordinals.pm b/util/perl/OpenSSL/Ordinals.pm
index 4b337c6ffd..f2517da7d8 100644
--- a/util/perl/OpenSSL/Ordinals.pm
+++ b/util/perl/OpenSSL/Ordinals.pm
@@ -130,7 +130,7 @@ sub load {
         s|#.*||;
         next if /^\s*$/;
 
-        my $item = OpenSSL::Ordinals::Item->new(from => $_);
+        my $item = OpenSSL::Ordinals::Item->new(source => $filename, from => $_);
 
         my $num = $item->number();
         if ($num eq '?') {
@@ -299,8 +299,10 @@ sub items {
 # Put an array of items back into the object after having checked consistency
 # If there are exactly two items:
 # - They MUST have the same number
+# - They MUST have the same version
 # - For platforms, both MUST hold the same ones, but with opposite values
 # - For features, both MUST hold the same ones.
+# - They MUST NOT have identical name, type, numeral, version, platforms, and features
 # If there's just one item, just put it in the slot of its number
 # In all other cases, something is wrong
 sub _putback {
@@ -308,8 +310,8 @@ sub _putback {
     my @items = @_;
 
     if (scalar @items < 1 || scalar @items > 2) {
-        croak "Wrong number of items: ", scalar @items, " : ",
-            join(", ", map { $_->name() } @items), "\n";
+        croak "Wrong number of items: ", scalar @items, "\n ",
+            join("\n ", map { $_->{source}.": ".$_->name() } @items), "\n";
     }
     if (scalar @items == 2) {
         # Collect some data
@@ -343,6 +345,13 @@ sub _putback {
             join(", ", sort keys %features), "\n"
             if %features;
 
+        # Check for in addition identical name, type, and platforms
+        croak "Duplicate entries for ".$items[0]->name()." from ".
+            $items[0]->source()." and ".$items[1]->source()."\n"
+            if $items[0]->name() eq $items[1]->name()
+            && $items[0]->type() eq $items[2]->type()
+            && $items[0]->platforms() eq $items[1]->platforms();
+
         # Check that all platforms exist in both items, and have opposite values
         my @platforms = ( { $items[0]->platforms() },
                           { $items[1]->platforms() } );
@@ -424,9 +433,10 @@ sub _adjust_version {
     return $version;
 }
 
-=item B<< $ordinals->add NAME, TYPE, LIST >>
+=item B<< $ordinals->add SOURCE, NAME, TYPE, LIST >>
 
-Adds a new item named NAME with the type TYPE, and a set of C macros in
+Adds a new item from file SOURCE named NAME with the type TYPE,
+and a set of C macros in
 LIST that are expected to be defined or undefined to use this symbol, if
 any.  For undefined macros, they each must be prefixed with a C<!>.
 
@@ -438,6 +448,7 @@ If it's entirely new, it will get a '?' and the current default version.
 
 sub add {
     my $self = shift;
+    my $source = shift;         # file where item was defined
     my $name = shift;
     my $type = shift;           # FUNCTION or VARIABLE
     my @defs = @_;              # Macros from #ifdef and #ifndef
@@ -462,7 +473,8 @@ sub add {
     @items = grep { $_->exists() } @items;
 
     my $new_item =
-        OpenSSL::Ordinals::Item->new( name          => $name,
+        OpenSSL::Ordinals::Item->new( source        => $source,
+                                      name          => $name,
                                       type          => $type,
                                       number        => $number,
                                       intnum        => $intnum,
@@ -485,15 +497,15 @@ sub add {
 
     # For the caller to show
     my @returns = ( $new_item );
-    push @returns, $self->add_alias($alias->{name}, $name, @{$alias->{defs}})
+    push @returns, $self->add_alias($source, $alias->{name}, $name, @{$alias->{defs}})
         if defined $alias;
     return @returns;
 }
 
-=item B<< $ordinals->add_alias ALIAS, NAME, LIST >>
+=item B<< $ordinals->add_alias SOURCE, ALIAS, NAME, LIST >>
 
-Adds an alias ALIAS for the symbol NAME, and a set of C macros in LIST
-that are expected to be defined or undefined to use this symbol, if any.
+Adds an alias ALIAS for the symbol NAME from file SOURCE, and a set of C macros
+in LIST that are expected to be defined or undefined to use this symbol, if any.
 For undefined macros, they each must be prefixed with a C<!>.
 
 If this symbol already exists in loaded data, it will be rewritten using
@@ -504,15 +516,16 @@ that the symbol NAME shows up.
 
 sub add_alias {
     my $self = shift;
+    my $source = shift;
     my $alias = shift;          # This is the alias being added
     my $name  = shift;          # For this name (assuming it exists)
     my @defs = @_;              # Platform attributes for the alias
 
     # call signature for debug output
     my $verbsig =
-        "add_alias('$alias' , '$name' , [ " . join(', ', @defs) . " ])";
+        "add_alias('$source' , '$alias' , '$name' , [ " . join(', ', @defs) . " ])";
 
-    croak "You're kidding me..." if $alias eq $name;
+    croak "You're kidding me... $alias == $name" if $alias eq $name;
 
     my %platforms = _parse_platforms(@defs);
     my %features = _parse_features(@defs);
@@ -533,7 +546,8 @@ sub add_alias {
     if (scalar @items == 0) {
         # The item we want to alias for doesn't exist yet, so we cache the
         # alias and hope the item we're making an alias of shows up later
-        $self->{aliases}->{$name} = { name => $alias, defs => [ @defs ] };
+        $self->{aliases}->{$name} = { source => $source,
+                                      name => $alias, defs => [ @defs ] };
 
         print STDERR "DEBUG[",__PACKAGE__,":add_alias] $verbsig\n",
             "\tSet future alias $alias => $name\n"
@@ -553,6 +567,7 @@ sub add_alias {
         my $number =
             $items[0]->number() =~ m|^\?| ? '?+' : $items[0]->number();
         my $alias_item = OpenSSL::Ordinals::Item->new(
+            source        => $source,
             name          => $alias,
             type          => $items[0]->type(),
             number        => $number,
@@ -734,9 +749,9 @@ Available options are:
 
 =over 4
 
-=item B<< from => STRING >>
+=item B<< source => FILENAME >>, B<< from => STRING >>
 
-This will create a new item, filled with data coming from STRING.
+This will create a new item from FILENAME, filled with data coming from STRING.
 
 STRING must conform to the following EBNF description:
 
@@ -757,8 +772,8 @@ STRING must conform to the following EBNF description:
 
 (C<letter> and C<digit> are assumed self evident)
 
-=item B<< name => STRING >>, B<< number => NUMBER >>, B<< version => STRING >>,
-      B<< exists => BOOLEAN >>, B<< type => STRING >>,
+=item B<< source => FILENAME >>, B<< name => STRING >>, B<< number => NUMBER >>,
+      B<< version => STRING >>, B<< exists => BOOLEAN >>, B<< type => STRING >>,
       B<< platforms => HASHref >>, B<< features => LISTref >>
 
 This will create a new item with data coming from the arguments.
@@ -796,7 +811,8 @@ sub new {
                                  /x );
 
         my @b = split /:/, $a[3];
-        %opts = ( name          => $a[0],
+        %opts = ( source        => $opts{source},
+                  name          => $a[0],
                   number        => $a[1],
                   version       => $a[2],
                   exists        => $b[0] eq 'EXIST',
@@ -812,7 +828,8 @@ sub new {
         my $version = $opts{version};
         $version =~ s|_|.|g;
 
-        $instance = { name      => $opts{name},
+        $instance = { source    => $opts{source},
+                      name      => $opts{name},
                       type      => $opts{type},
                       number    => $opts{number},
                       intnum    => $opts{intnum},