On Sun Mar 29, 2020 at 10:56:09PM +0200, Matthias Kilian wrote:
> Hi,
>
> this hopefully fixes the segfaults some people did see. It's the
> patch I dropped from the previous poppler update plus another one.
>
> Because a full update to the next poppler version (0.87.0) includes
> yet another API breakage (some constification in SplashScreen), I
> prefer to rush only those two patches in for now (building and
> probably patching poppler consumers takes some time for me).
>
> I'm not sure about the nimor bump to libpoppler.so --
> /usr/src/lib/check_sym reports some added and some strengthened
> symbols. If this requires a major bump, please let me know.
>
Yeah, the new destructors create new symbols but that's not a problem.
>From reading the diffs I see no concerns. If nobody scream up, OK rsadowski@
> Ciao,
> Kili
>
> ps: please note that this is only build-tested for now (me sitting at
> home, my build machine at work, shitty internet connection with lots of
> package loss)
>
> pps: sorry for the breakage -- I thought that only one of my broken
> test PDF documents was affected (all other ones didn't trigger the
> bug).
>
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/print/poppler/Makefile,v
> retrieving revision 1.155
> diff -u -p -r1.155 Makefile
> --- Makefile 21 Mar 2020 22:42:42 -0000 1.155
> +++ Makefile 29 Mar 2020 20:43:09 -0000
> @@ -10,13 +10,14 @@ CATEGORIES= print
> PKGNAME-main= poppler-$V
> PKGNAME-utils= poppler-utils-$V
> PKGNAME-qt5= poppler-qt5-$V
> -REVISION-main= 0
> -REVISION-utils= 0
> +REVISION-main= 1
> +REVISION-glib= 0
> +REVISION-utils= 1
> REVISION-qt5= 0
>
> EXTRACT_SUFX= .tar.xz
>
> -SHARED_LIBS += poppler 61.0 # 97.0
> +SHARED_LIBS += poppler 61.1 # 97.0
> SHARED_LIBS += poppler-glib 19.4 # 8.15
> SHARED_LIBS += poppler-qt5 8.2 # 1.22
> SHARED_LIBS += poppler-cpp 16.0 # 0.7
> Index: patches/patch-glib_poppler-action_cc
> ===================================================================
> RCS file: patches/patch-glib_poppler-action_cc
> diff -N patches/patch-glib_poppler-action_cc
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-glib_poppler-action_cc 29 Mar 2020 20:43:09 -0000
> @@ -0,0 +1,60 @@
> +$OpenBSD$
> +
> +Upstream commit 68b6dd2ecd868c1a757cb8b9273e2e26687e5229 (Adam
> +Reichold):
> +
> +Replace dynamic_cast by static_cast where we already perform the
> +type checks explicitly before downcasting.
> +
> +Index: glib/poppler-action.cc
> +--- glib/poppler-action.cc.orig
> ++++ glib/poppler-action.cc
> +@@ -627,39 +627,39 @@ _poppler_action_new (PopplerDocument *document,
> + switch (link->getKind ()) {
> + case actionGoTo:
> + action->type = POPPLER_ACTION_GOTO_DEST;
> +- build_goto_dest (document, action, dynamic_cast <const LinkGoTo
> *> (link));
> ++ build_goto_dest (document, action, static_cast <const LinkGoTo
> *> (link));
> + break;
> + case actionGoToR:
> + action->type = POPPLER_ACTION_GOTO_REMOTE;
> +- build_goto_remote (action, dynamic_cast <const LinkGoToR *>
> (link));
> ++ build_goto_remote (action, static_cast <const LinkGoToR *>
> (link));
> + break;
> + case actionLaunch:
> + action->type = POPPLER_ACTION_LAUNCH;
> +- build_launch (action, dynamic_cast <const LinkLaunch *> (link));
> ++ build_launch (action, static_cast <const LinkLaunch *> (link));
> + break;
> + case actionURI:
> + action->type = POPPLER_ACTION_URI;
> +- build_uri (action, dynamic_cast <const LinkURI *> (link));
> ++ build_uri (action, static_cast <const LinkURI *> (link));
> + break;
> + case actionNamed:
> + action->type = POPPLER_ACTION_NAMED;
> +- build_named (action, dynamic_cast <const LinkNamed *> (link));
> ++ build_named (action, static_cast <const LinkNamed *> (link));
> + break;
> + case actionMovie:
> + action->type = POPPLER_ACTION_MOVIE;
> +- build_movie (document, action, dynamic_cast<const LinkMovie*>
> (link));
> ++ build_movie (document, action, static_cast<const LinkMovie*>
> (link));
> + break;
> + case actionRendition:
> + action->type = POPPLER_ACTION_RENDITION;
> +- build_rendition (action, dynamic_cast<const LinkRendition*>
> (link));
> ++ build_rendition (action, static_cast<const LinkRendition*>
> (link));
> + break;
> + case actionOCGState:
> + action->type = POPPLER_ACTION_OCG_STATE;
> +- build_ocg_state (document, action, dynamic_cast<const
> LinkOCGState*> (link));
> ++ build_ocg_state (document, action, static_cast<const
> LinkOCGState*> (link));
> + break;
> + case actionJavaScript:
> + action->type = POPPLER_ACTION_JAVASCRIPT;
> +- build_javascript (action, dynamic_cast<const LinkJavaScript*>
> (link));
> ++ build_javascript (action, static_cast<const LinkJavaScript*>
> (link));
> + break;
> + case actionUnknown:
> + default:
> Index: patches/patch-poppler_Link_cc
> ===================================================================
> RCS file: patches/patch-poppler_Link_cc
> diff -N patches/patch-poppler_Link_cc
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-poppler_Link_cc 29 Mar 2020 20:43:09 -0000
> @@ -0,0 +1,108 @@
> +$OpenBSD$
> +
> +Upstream commit 81a86064c14a7fc25047b6040d65464e732cf501 (Adam
> +Reichold):
> +
> +Fix vague linkage of Link* class vtables
> +
> +Due to falling back to the implicitly inline destructors, some of
> +the Link* classes had all their overridden methods defined inline
> +with made the linkage of their vtables vague.
> +
> +This change moves their destructors into a defined translation unit
> +thereby anchoring their vtables in the libpoppler DSO which fixes
> +issues using dynamic_cast when builing Poppler using Clang.
> +
> +Index: poppler/Link.cc
> +--- poppler/Link.cc.orig
> ++++ poppler/Link.cc
> +@@ -444,6 +444,8 @@ LinkGoTo::LinkGoTo(const Object *destObj) {
> + }
> + }
> +
> ++LinkGoTo::~LinkGoTo() = default;
> ++
> + //------------------------------------------------------------------------
> + // LinkGoToR
> + //------------------------------------------------------------------------
> +@@ -474,6 +476,8 @@ LinkGoToR::LinkGoToR(Object *fileSpecObj, Object *dest
> + }
> + }
> +
> ++LinkGoToR::~LinkGoToR() = default;
> ++
> + //------------------------------------------------------------------------
> + // LinkLaunch
> + //------------------------------------------------------------------------
> +@@ -552,6 +556,8 @@ LinkURI::LinkURI(const Object *uriObj, const GooString
> + }
> + }
> +
> ++LinkURI::~LinkURI() = default;
> ++
> + //------------------------------------------------------------------------
> + // LinkNamed
> + //------------------------------------------------------------------------
> +@@ -564,6 +570,8 @@ LinkNamed::LinkNamed(const Object *nameObj) {
> + }
> + }
> +
> ++LinkNamed::~LinkNamed() = default;
> ++
> + //------------------------------------------------------------------------
> + // LinkMovie
> + //------------------------------------------------------------------------
> +@@ -607,6 +615,8 @@ LinkMovie::LinkMovie(const Object *obj) {
> + }
> + }
> +
> ++LinkMovie::~LinkMovie() = default;
> ++
> + //------------------------------------------------------------------------
> + // LinkSound
> + //------------------------------------------------------------------------
> +@@ -645,6 +655,8 @@ LinkSound::LinkSound(const Object *soundObj) {
> + }
> + }
> +
> ++LinkSound::~LinkSound() = default;
> ++
> + //------------------------------------------------------------------------
> + // LinkRendition
> + //------------------------------------------------------------------------
> +@@ -738,6 +750,8 @@ LinkJavaScript::LinkJavaScript(Object *jsObj) {
> + }
> + }
> +
> ++LinkJavaScript::~LinkJavaScript() = default;
> ++
> + Object LinkJavaScript::createObject(XRef *xref, const GooString &js)
> + {
> + Dict *linkDict = new Dict(xref);
> +@@ -793,6 +807,8 @@ LinkOCGState::LinkOCGState(const Object *obj)
> + preserveRB = obj->dictLookup("PreserveRB").getBoolWithDefaultValue(true);
> + }
> +
> ++LinkOCGState::~LinkOCGState() = default;
> ++
> + //------------------------------------------------------------------------
> + // LinkHide
> + //------------------------------------------------------------------------
> +@@ -814,6 +830,8 @@ LinkHide::LinkHide(const Object *hideObj) {
> + }
> + }
> +
> ++LinkHide::~LinkHide() = default;
> ++
> + //------------------------------------------------------------------------
> + // LinkUnknown
> + //------------------------------------------------------------------------
> +@@ -821,6 +839,8 @@ LinkHide::LinkHide(const Object *hideObj) {
> + LinkUnknown::LinkUnknown(const char *actionA) {
> + action = std::string(actionA ? actionA : "");
> + }
> ++
> ++LinkUnknown::~LinkUnknown() = default;
> +
> + //------------------------------------------------------------------------
> + // Links
> Index: patches/patch-poppler_Link_h
> ===================================================================
> RCS file: patches/patch-poppler_Link_h
> diff -N patches/patch-poppler_Link_h
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-poppler_Link_h 29 Mar 2020 20:43:09 -0000
> @@ -0,0 +1,108 @@
> +$OpenBSD$
> +
> +Upstream commit 81a86064c14a7fc25047b6040d65464e732cf501 (Adam
> +Reichold):
> +
> +Fix vague linkage of Link* class vtables
> +
> +Due to falling back to the implicitly inline destructors, some of
> +the Link* classes had all their overridden methods defined inline
> +with made the linkage of their vtables vague.
> +
> +This change moves their destructors into a defined translation unit
> +thereby anchoring their vtables in the libpoppler DSO which fixes
> +issues using dynamic_cast when builing Poppler using Clang.
> +
> +Index: poppler/Link.h
> +--- poppler/Link.h.orig
> ++++ poppler/Link.h
> +@@ -164,6 +164,8 @@ class LinkGoTo: public LinkAction { (public)
> + // Build a LinkGoTo from a destination (dictionary, name, or string).
> + LinkGoTo(const Object *destObj);
> +
> ++ ~LinkGoTo() override;
> ++
> + // Was the LinkGoTo created successfully?
> + bool isOk() const override { return dest || namedDest; }
> +
> +@@ -191,6 +193,8 @@ class LinkGoToR: public LinkAction { (public)
> + // (dictionary, name, or string).
> + LinkGoToR(Object *fileSpecObj, Object *destObj);
> +
> ++ ~LinkGoToR() override;
> ++
> + // Was the LinkGoToR created successfully?
> + bool isOk() const override { return fileName && (dest || namedDest); }
> +
> +@@ -243,6 +247,8 @@ class LinkURI: public LinkAction { (public)
> + // Build a LinkURI given the URI (string) and base URI.
> + LinkURI(const Object *uriObj, const GooString *baseURI);
> +
> ++ ~LinkURI() override;
> ++
> + // Was the LinkURI created successfully?
> + bool isOk() const override { return hasURIFlag; }
> +
> +@@ -266,6 +272,8 @@ class LinkNamed: public LinkAction { (public)
> + // Build a LinkNamed given the action name.
> + LinkNamed(const Object *nameObj);
> +
> ++ ~LinkNamed() override;
> ++
> + bool isOk() const override { return hasNameFlag; }
> +
> + LinkActionKind getKind() const override { return actionNamed; }
> +@@ -294,6 +302,8 @@ class LinkMovie: public LinkAction { (public)
> +
> + LinkMovie(const Object *obj);
> +
> ++ ~LinkMovie() override;
> ++
> + bool isOk() const override { return hasAnnotRef() || hasAnnotTitleFlag; }
> + LinkActionKind getKind() const override { return actionMovie; }
> +
> +@@ -374,6 +384,8 @@ class LinkSound: public LinkAction { (public)
> +
> + LinkSound(const Object *soundObj);
> +
> ++ ~LinkSound() override;
> ++
> + bool isOk() const override { return sound != nullptr; }
> +
> + LinkActionKind getKind() const override { return actionSound; }
> +@@ -403,6 +415,8 @@ class LinkJavaScript: public LinkAction { (public)
> + // Build a LinkJavaScript given the action name.
> + LinkJavaScript(Object *jsObj);
> +
> ++ ~LinkJavaScript() override;
> ++
> + bool isOk() const override { return isValid; }
> +
> + LinkActionKind getKind() const override { return actionJavaScript; }
> +@@ -423,7 +437,7 @@ class LinkOCGState: public LinkAction {
> + public:
> + LinkOCGState(const Object *obj);
> +
> +- ~LinkOCGState() override = default;
> ++ ~LinkOCGState() override;
> +
> + bool isOk() const override { return isValid; }
> +
> +@@ -454,6 +468,8 @@ class LinkHide: public LinkAction {
> + public:
> + LinkHide(const Object *hideObj);
> +
> ++ ~LinkHide() override;
> ++
> + bool isOk() const override { return hasTargetNameFlag; }
> + LinkActionKind getKind() const override { return actionHide; }
> +
> +@@ -488,6 +504,8 @@ class LinkUnknown: public LinkAction { (public)
> +
> + // Build a LinkUnknown with the specified action type.
> + LinkUnknown(const char *actionA);
> ++
> ++ ~LinkUnknown() override;
> +
> + // Was the LinkUnknown create successfully?
> + // Yes: nothing can go wrong when creating LinkUnknown objects
> Index: patches/patch-utils_HtmlOutputDev_cc
> ===================================================================
> RCS file: patches/patch-utils_HtmlOutputDev_cc
> diff -N patches/patch-utils_HtmlOutputDev_cc
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-utils_HtmlOutputDev_cc 29 Mar 2020 20:43:09 -0000
> @@ -0,0 +1,20 @@
> +$OpenBSD$
> +
> +Upstream commit 68b6dd2ecd868c1a757cb8b9273e2e26687e5229 (Adam
> +Reichold):
> +
> +Replace dynamic_cast by static_cast where we already perform the
> +type checks explicitly before downcasting.
> +
> +Index: utils/HtmlOutputDev.cc
> +--- utils/HtmlOutputDev.cc.orig
> ++++ utils/HtmlOutputDev.cc
> +@@ -1838,7 +1838,7 @@ int HtmlOutputDev::getOutlinePageNum(OutlineItem *item
> + if (!action || action->getKind() != actionGoTo)
> + return pagenum;
> +
> +- link = dynamic_cast<const LinkGoTo*>(action);
> ++ link = static_cast<const LinkGoTo*>(action);
> +
> + if (!link || !link->isOk())
> + return pagenum;
>
