New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 673778 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Cr51 targets fail due to #error in sha384.c

Project Member Reported by carlh@chromium.org, Dec 13 2016

Issue description

Error output from compiler:

sha384.c:22:2: error: #error "-DSHA512_SUPPORT must be specified to enable SHA-384/512"
 #error "-DSHA512_SUPPORT must be specified to enable SHA-384/512"


sha384.c was introduced to third_party/cryptoc in: https://chromium-review.googlesource.com/#/c/418263/

The Makefile changes ensure that the sha384.c is only compiled if SHA512_SUPPORT is defined. The file itself ensures that SHA512_SUPPORT is defined with an #error pre-processor directive.

Unfortunately, chip/g/build.mk compiles sha384.c unconditionally but doesn't add SHA512_SUPPORT to the list of manifest constants defined on the compiler command line.

board/cr50/build.mk does add SHA512_SUPPORT to the list of manifest constants, so the cr50 build succeeds.

However, there are other boards using third_party/cryptoc via chip/g/build.mk. Most notably those in private-cr51. These builds are currently failing.

I don't know if the right answer is to add SHA512_SUPPORT in chip/g/build.mk or to conditionally compile sha384.c (and sha512.c) in chip/g/build.mk.


 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/c7ea2c9125b9d6be9a52c6358e2892ff67e404c7

commit c7ea2c9125b9d6be9a52c6358e2892ff67e404c7
Author: nagendra modadugu <ngm@google.com>
Date: Tue Dec 13 18:20:54 2016

CR50: make SHA512 a config option

Turn SHA512 support into a config option so that
boards may individually enable SHA512 support.

BRANCH=none
BUG=chromium:673778
CQ-DEPEND=CL:419578
TEST=make buildall succeeds

Change-Id: Ib857a3e97f1c2ec7066ae23ac725c7bf3d194e01
Signed-off-by: nagendra modadugu <ngm@google.com>
Reviewed-on: https://chromium-review.googlesource.com/419327
Commit-Ready: Nagendra Modadugu <ngm@google.com>
Tested-by: Nagendra Modadugu <ngm@google.com>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>

[modify] https://crrev.com/c7ea2c9125b9d6be9a52c6358e2892ff67e404c7/board/cr50/board.h
[modify] https://crrev.com/c7ea2c9125b9d6be9a52c6358e2892ff67e404c7/include/config.h
[modify] https://crrev.com/c7ea2c9125b9d6be9a52c6358e2892ff67e404c7/chip/g/build.mk
[modify] https://crrev.com/c7ea2c9125b9d6be9a52c6358e2892ff67e404c7/board/cr50/build.mk

Comment 2 by ngm@chromium.org, Dec 14 2016

Vadim, is there something that can be done for the submit queue to catch this?  Is cr51 etc. not part of the default setup?
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 15 2016

Labels: merge-merged-firmware-reef-9042.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/b71c06da2f3a758076bebdc09351e77114015b66

commit b71c06da2f3a758076bebdc09351e77114015b66
Author: nagendra modadugu <ngm@google.com>
Date: Tue Dec 13 18:20:54 2016

CR50: make SHA512 a config option

Turn SHA512 support into a config option so that
boards may individually enable SHA512 support.

BRANCH=none
BUG=chromium:673778
CQ-DEPEND=CL:419578
TEST=make buildall succeeds

Change-Id: Ib857a3e97f1c2ec7066ae23ac725c7bf3d194e01
Signed-off-by: nagendra modadugu <ngm@google.com>
Reviewed-on: https://chromium-review.googlesource.com/419327
Commit-Ready: Nagendra Modadugu <ngm@google.com>
Tested-by: Nagendra Modadugu <ngm@google.com>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/420776
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Commit-Queue: Aaron Durbin <adurbin@chromium.org>
Tested-by: Aaron Durbin <adurbin@chromium.org>

[modify] https://crrev.com/b71c06da2f3a758076bebdc09351e77114015b66/board/cr50/board.h
[modify] https://crrev.com/b71c06da2f3a758076bebdc09351e77114015b66/include/config.h
[modify] https://crrev.com/b71c06da2f3a758076bebdc09351e77114015b66/chip/g/build.mk
[modify] https://crrev.com/b71c06da2f3a758076bebdc09351e77114015b66/board/cr50/build.mk

Status: Assigned (was: Untriaged)

Sign in to add a comment