New issue
Advanced search Search tips

Issue 845505 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 843209
issue 844432



Sign in to add a comment

Coverage Blocker: skia_shared SkFloatToDecimal.cpp function definition conflicts with root skia SkFloatToDecimal.cpp

Project Member Reported by infe...@chromium.org, May 22 2018

Issue description

See https://bugs.chromium.org/p/chromium/issues/detail?id=844432#c1

pow10 function has different definitions when dchecks are on (using dcheck_is_configurable=true) in release mode, so llvm coverage fails. pow10 in skia will use SkAssert, while your SkAssert won't be enabled since it is #define to assert (only in debug).

We need to enable dcheck_is_configurable=true for https://bugs.chromium.org/p/chromium/issues/detail?id=843209

Easy fix is move your pow10 function inside the pdfium::skia namespace

namespace pdfium {
namespace skia {

// Return pow(10.0, e), optimized for common cases.
static double pow10(int e) {

That way, it tells llvm coverage that this is a different symbol. Or another fix is to rename pow10 to something different if you want to keep in global context.
 
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, May 23 2018

The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium/+/529b13ac5d092bbc195e69fe2ba58c0e5b9da4b7

commit 529b13ac5d092bbc195e69fe2ba58c0e5b9da4b7
Author: Dan Sinclair <dsinclair@chromium.org>
Date: Wed May 23 14:50:40 2018

Move SkFloatToDecimal pow10 inside anonymous namespace

This CL moves the pow10 definition inside the pdfium::skia namespace in
order to fix conflicts with the skia version of the same file. This
comes up due to the code coverage tools.

Bug:  chromium:845505 
Change-Id: Ief53a2bbd15cb9828be23417ff3442461347d146
Reviewed-on: https://pdfium-review.googlesource.com/32893
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>

[modify] https://crrev.com/529b13ac5d092bbc195e69fe2ba58c0e5b9da4b7/third_party/skia_shared/SkFloatToDecimal.cpp

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, May 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6853782ce5e07ef4ff71d5f71d89ca6c40fac5d8

commit 6853782ce5e07ef4ff71d5f71d89ca6c40fac5d8
Author: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed May 23 19:11:56 2018

Roll src/third_party/pdfium/ 7f472cd8a..d0f10a824 (5 commits)

https://pdfium.googlesource.com/pdfium.git/+log/7f472cd8abe4..d0f10a8240c2

$ git log 7f472cd8a..d0f10a824 --date=short --no-merges --format='%ad %ae %s'
2018-05-23 thestig Fix linking errors in fuzzer targets.
2018-05-23 dsinclair Move SkFloatToDecimal pow10 inside anonymous namespace
2018-05-22 thestig Make friend RetainPtr<T> statements consistently public.
2018-05-22 thestig Fix nits in fxjs.
2018-05-22 thestig Simplify CPDF_ShadingPattern::Validate().

Created with:
  roll-dep src/third_party/pdfium
BUG= chromium:845771 , chromium:845505 


The AutoRoll server is located here: https://pdfium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


TBR=dsinclair@chromium.org

Change-Id: I69f400d567a3d72c93161d10f2ecc57ec7b2d8fa
Reviewed-on: https://chromium-review.googlesource.com/1067687
Reviewed-by: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#561193}
[modify] https://crrev.com/6853782ce5e07ef4ff71d5f71d89ca6c40fac5d8/DEPS

Sign in to add a comment