[gdal-dev] /MD vs /MDd for DEBUG MSVC builds

Discussion:

Even Rouault

2017-09-27 19:38:15 UTC

Hi,

For those who don't understand the title, you may skip this email ;-)

Once again someone raised an issue about the MSVC runtime we use for DEBUG=1 builds:
https://trac.osgeo.org/gdal/ticket/7059

and in https://trac.osgeo.org/gdal/ticket/6384 about the same issue, there are 4 similar
tickets raised.

So it seems our default is a recurring problem.

Personnaly I believe I've never used a Windows build, so I'm mostly indifferent

Any opinion about using the debug MSVC runtime (/MDd) for DEBUG=1 builds instead of the
current release runtime (/MD) ?

Even

--
Spatialys - Geospatial professional services
http://www.spatialys.com

Mateusz Loskot

2017-09-27 19:52:45 UTC

Permalink

Post by Even Rouault
Once again someone raised an issue about the MSVC runtime we use for DEBUG=1
builds: https://trac.osgeo.org/gdal/ticket/7059
[...]
Personnaly I believe I've never used a Windows build, so I'm mostly indifferent
Any opinion about using the debug MSVC runtime (/MDd) for DEBUG=1 builds
instead of the current release runtime (/MD) ?

Even,

Yesterday, I started thinking about adding an extra build configuration to NMAKE
makefile, controlled via command line defines, to enable such proper/full debug
build for Windows.

IMO, debug builds w/o /MDd are somewhat unexpected or exotic.
As one of Windows users and developers, I'd vote for the default
MSVC configuration for debug builds of GDAL.

This would also help a bit with binary packages for Visual C++,
currently, it requires nmake.opt patching.
https://github.com/Microsoft/vcpkg/pull/1879

Best regards,

--
Mateusz Loskot, http://mateusz.loskot.net

Jeff McKenna

2017-09-27 22:10:16 UTC

Permalink

Post by Mateusz Loskot

Even,
Yesterday, I started thinking about adding an extra build configuration to NMAKE
makefile, controlled via command line defines, to enable such proper/full debug
build for Windows.
IMO, debug builds w/o /MDd are somewhat unexpected or exotic.
As one of Windows users and developers, I'd vote for the default
MSVC configuration for debug builds of GDAL.
This would also help a bit with binary packages for Visual C++,
currently, it requires nmake.opt patching.
https://github.com/Microsoft/vcpkg/pull/1879
Best regards,

Mateusz Loskot

2017-09-27 22:20:34 UTC

Permalink

Post by Mateusz Loskot

Even,
Yesterday, I started thinking about adding an extra build configuration to NMAKE
makefile, controlled via command line defines, to enable such proper/full debug
build for Windows.
IMO, debug builds w/o /MDd are somewhat unexpected or exotic.
As one of Windows users and developers, I'd vote for the default
MSVC configuration for debug builds of GDAL.
This would also help a bit with binary packages for Visual C++,
currently, it requires nmake.opt patching.
https://github.com/Microsoft/vcpkg/pull/1879
Best regards,

To clarify: Mateusz you mean that debug builds should be changed to /MDd,
correct? I somehow read your response as you also want to change the
default build in nmake to be the debug build. Or, maybe my brain is tired
ha :) -jeff

Jeff, yes that is what I was trying to say, to change the debug defaults
only.

Perhaps I shouldn't paste the link to vcpkg as the flags patched there are
different, not relevant here.

However, one may want to also package and offer debug binaries for MSVC
like Python 3.5 or 3.6+, and /MDd would definitely be a must as that is the
MSVC way.

Mat

Jeff McKenna

2017-09-27 22:05:12 UTC

Permalink

Post by Even Rouault
Hi,
For those who don't understand the title, you may skip this email ;-)
https://trac.osgeo.org/gdal/ticket/7059
and in https://trac.osgeo.org/gdal/ticket/6384 about the same issue,
there are 4 similar tickets raised.
So it seems our default is a recurring problem.
Personnaly I believe I've never used a Windows build, so I'm mostly indifferent
Any opinion about using the debug MSVC runtime (/MDd) for DEBUG=1 builds
instead of the current release runtime (/MD) ?
Even

The fix proposed in ticket 7059 makes sense, I see the problem here in
my local nmake, so +1

But I don't understand Mateusz's response. I think the change in ticket
7059 should be made, and no changes to any defaults, because packagers
by default are not sharing default builds. Again, maybe I am
misunderstanding Mateusz's message.

-jeff

--
Jeff McKenna
MapServer Consulting and Training Services
http://www.gatewaygeomatics.com/

Even Rouault

2017-09-27 22:11:12 UTC

Permalink

Post by Jeff McKenna

The fix proposed in ticket 7059 makes sense, I see the problem here in
my local nmake, so +1
But I don't understand Mateusz's response. I think the change in ticket
7059 should be made, and no changes to any defaults, because packagers
by default are not sharing default builds. Again, maybe I am
misunderstanding Mateusz's message.

So things are clear, I was thinking to this change

$ svn diff nmake.opt
Index: nmake.opt
===================================================================
--- nmake.opt (revision 40206)
+++ nmake.opt (working copy)
@@ -132,13 +132,13 @@
!IFNDEF DEBUG
OPTFLAGS= $(CXX_ANALYZE_FLAGS) $(CXX_PDB_FLAGS) /nologo /MP /MD /EHsc /Ox /FC /D_CRT_SECURE_NO_DEPRECATE /D_CRT_NONSTDC_NO_DEPRECATE /DNDEBUG
!ELSE
-OPTFLAGS= $(CXX_ANALYZE_FLAGS) $(CXX_PDB_FLAGS) /nologo /MP /MD /EHsc /FC /D_CRT_SECURE_NO_DEPRECATE /D_CRT_NONSTDC_NO_DEPRECATE /DDEBUG
+OPTFLAGS= $(CXX_ANALYZE_FLAGS) $(CXX_PDB_FLAGS) /nologo /MP /MDd /EHsc /FC /D_CRT_SECURE_NO_DEPRECATE /D_CRT_NONSTDC_NO_DEPRECATE /DDEBUG
!ENDIF
!ELSE
!IFNDEF DEBUG
OPTFLAGS= $(CXX_PDB_FLAGS) /nologo /MD /EHsc /GR /Ox /FC /DNDEBUG
!ELSE
-OPTFLAGS= $(CXX_PDB_FLAGS) /nologo /MD /EHsc /GR /FC /DDEBUG
+OPTFLAGS= $(CXX_PDB_FLAGS) /nologo /MDd /EHsc /GR /FC /DDEBUG
!ENDIF
!ENDIF #MSVC_VER
!ENDIF # OPTFLAGS

(ah and I mistyped my initial message. I have sometimes done Windows builds,
just not *debug* builds)

--
Spatialys - Geospatial professional services
http://www.spatialys.com

Even Rouault

2017-09-28 10:05:45 UTC

Permalink

Hi,

Given the feedback, I've just committed the below change (in trunk only)

Even

Post by Even Rouault
$ svn diff nmake.opt
Index: nmake.opt
===================================================================
--- nmake.opt (revision 40206)
+++ nmake.opt (working copy)
@@ -132,13 +132,13 @@
!IFNDEF DEBUG
OPTFLAGS= $(CXX_ANALYZE_FLAGS) $(CXX_PDB_FLAGS) /nologo /MP /MD /EHsc /Ox
/FC /D_CRT_SECURE_NO_DEPRECATE /D_CRT_NONSTDC_NO_DEPRECATE /DNDEBUG !

ELSE

Post by Even Rouault
-OPTFLAGS= $(CXX_ANALYZE_FLAGS) $(CXX_PDB_FLAGS) /nologo /MP /MD /EHsc /FC
/D_CRT_SECURE_NO_DEPRECATE /D_CRT_NONSTDC_NO_DEPRECATE /DDEBUG

+OPTFLAGS=

Post by Even Rouault
$(CXX_ANALYZE_FLAGS) $(CXX_PDB_FLAGS) /nologo /MP /MDd /EHsc /FC
/D_CRT_SECURE_NO_DEPRECATE /D_CRT_NONSTDC_NO_DEPRECATE /DDEBUG !ENDIF
!ELSE
!IFNDEF DEBUG
OPTFLAGS= $(CXX_PDB_FLAGS) /nologo /MD /EHsc /GR /Ox /FC /DNDEBUG
!ELSE
-OPTFLAGS= $(CXX_PDB_FLAGS) /nologo /MD /EHsc /GR /FC /DDEBUG
+OPTFLAGS= $(CXX_PDB_FLAGS) /nologo /MDd /EHsc /GR /FC /DDEBUG
!ENDIF
!ENDIF #MSVC_VER
!ENDIF # OPTFLAGS

--
Spatialys - Geospatial professional services
http://www.spatialys.com

Mateusz Loskot

2017-09-28 10:21:59 UTC

Permalink

Post by Even Rouault
Given the feedback, I've just committed the below change (in trunk only)
-OPTFLAGS= $(CXX_PDB_FLAGS) /nologo /MD /EHsc /GR /FC /DDEBUG
+OPTFLAGS= $(CXX_PDB_FLAGS) /nologo /MDd /EHsc /GR /FC /DDEBUG

It's worth to notice that switch to /MDd also automatically defines _DEBUG [1].

[1] https://docs.microsoft.com/en-us/cpp/c-runtime-library/debug

--
Mateusz Loskot, http://mateusz.loskot.net

Tamas Szekeres

2017-10-16 21:01:52 UTC

Permalink

Looks like I've missed this thread earlier, but according to this change we
might either compile all the dependent libraries for /MDd (at least for the
statically linked libraries) or we trust in that GDAL is safe to compile
against a different CRT than the dependencies. That means that GDAL won't
free up memory that have been allocated in either of the dependencies or
vica versa. I'm not completely sure if the latter applies.

The earlier approach was a bit more like the RelWithDebInfo setting in the
cmake terminology which is not considered as a wrong setting, but that has
it's own purpose. At the moment I'm not aware of any binary distributions
or SDKs out of the box which would be compatible with the /MDd setting,
that causes that DEBUG=1 has a fairly limited usability from now on.

Best regards,

Tamas

Post by Even Rouault
Hi,
Given the feedback, I've just committed the below change (in trunk only)
Even

/FC

Post by Even Rouault
/D_CRT_SECURE_NO_DEPRECATE /D_CRT_NONSTDC_NO_DEPRECATE /DDEBUG +OPTFLAGS=
$(CXX_ANALYZE_FLAGS) $(CXX_PDB_FLAGS) /nologo /MP /MDd /EHsc /FC
/D_CRT_SECURE_NO_DEPRECATE /D_CRT_NONSTDC_NO_DEPRECATE /DDEBUG !ENDIF
!ELSE
!IFNDEF DEBUG
OPTFLAGS= $(CXX_PDB_FLAGS) /nologo /MD /EHsc /GR /Ox /FC /DNDEBUG
!ELSE
-OPTFLAGS= $(CXX_PDB_FLAGS) /nologo /MD /EHsc /GR /FC /DDEBUG
+OPTFLAGS= $(CXX_PDB_FLAGS) /nologo /MDd /EHsc /GR /FC /DDEBUG
!ENDIF
!ENDIF #MSVC_VER
!ENDIF # OPTFLAGS

--
Spatialys - Geospatial professional services
http://www.spatialys.com
_______________________________________________
gdal-dev mailing list
https://lists.osgeo.org/mailman/listinfo/gdal-dev

Even Rouault

2017-10-16 21:20:11 UTC

Permalink

Post by Tamas Szekeres
Looks like I've missed this thread earlier, but according to this change we
might either compile all the dependent libraries for /MDd (at least for the
statically linked libraries) or we trust in that GDAL is safe to compile
against a different CRT than the dependencies. That means that GDAL won't
free up memory that have been allocated in either of the dependencies or
vica versa. I'm not completely sure if the latter applies.
The earlier approach was a bit more like the RelWithDebInfo setting in the
cmake terminology which is not considered as a wrong setting, but that has
it's own purpose. At the moment I'm not aware of any binary distributions
or SDKs out of the box which would be compatible with the /MDd setting,
that causes that DEBUG=1 has a fairly limited usability from now on.

--
Spatialys - Geospatial professional services
http://www.spatialys.com

Mateusz Loskot

2017-10-16 21:48:07 UTC

Permalink

Ah Windows...

I guess the people who complained did builds with none or little
dependencies.

Perhaps adding a RELWITHDEBINFO=1 flag that would expand to

OPTFLAGS= $(CXX_ANALYZE_FLAGS) $(CXX_PDB_FLAGS) /nologo /MP /MD /EHsc /FC /
D_CRT_SECURE_NO_DEPRECATE /D_CRT_NONSTDC_NO_DEPRECATE /DDEBUG

(ie same as default build but without /Ox and with /DDEBUG)

would help ?

That would not be equivalent to CMake RelWithDebugInfo, which is is pretty
self-descriptive. Here is explanation from
https://cmake.org/pipermail/cmake/2001-October/002479.html

"The difference between Debug and RelwithDebInfo is that RelwithDebInfo is
quite similar to Release mode. It produces fully optimised code, but also
builds the program database, and inserts debug line information..."

Or, I'm m being unclear about use of /DEBUG

Mateusz

Even Rouault

2017-10-16 21:55:56 UTC

Permalink

Post by Mateusz Loskot
That would not be equivalent to CMake RelWithDebugInfo, which is is pretty
self-descriptive. Here is explanation from
https://cmake.org/pipermail/cmake/2001-October/002479.html
"The difference between Debug and RelwithDebInfo is that RelwithDebInfo is
quite similar to Release mode. It produces fully optimised code, but also
builds the program database, and inserts debug line information..."
Or, I'm m being unclear about use of /DEBUG

I didn't check CMake semantics. Hum, then what about calling it
DEBUG_WITH_RELEASE_CRT=1 ?

--
Spatialys - Geospatial professional services
http://www.spatialys.com

Tamas Szekeres

2017-10-16 22:30:54 UTC

Permalink

I'd like to see /Od in addition to the /Zi and /MD etc. with the
DEBUG_WITH_RELEASE_CRT
setting. By this means it's not the same as RelWithDebInfo which uses /O2
as far as I remember, which was the only setting that I always have to
change with the cmake builds to make it usable for debugging purposes.

Best regards,

Tamas

Post by Mateusz Loskot

Post by Mateusz Loskot
That would not be equivalent to CMake RelWithDebugInfo, which is is

pretty

Post by Mateusz Loskot
self-descriptive. Here is explanation from
https://cmake.org/pipermail/cmake/2001-October/002479.html
"The difference between Debug and RelwithDebInfo is that RelwithDebInfo

Post by Mateusz Loskot
quite similar to Release mode. It produces fully optimised code, but also
builds the program database, and inserts debug line information..."
Or, I'm m being unclear about use of /DEBUG

I didn't check CMake semantics. Hum, then what about calling it
DEBUG_WITH_RELEASE_CRT=1 ?
--
Spatialys - Geospatial professional services
http://www.spatialys.com
_______________________________________________
gdal-dev mailing list
https://lists.osgeo.org/mailman/listinfo/gdal-dev

Even Rouault

2017-10-16 22:35:21 UTC

Permalink

Post by Tamas Szekeres
I'd like to see /Od in addition to the /Zi and /MD etc. with the
DEBUG_WITH_RELEASE_CRT
setting. By this means it's not the same as RelWithDebInfo which uses /O2
as far as I remember, which was the only setting that I always have to
change with the cmake builds to make it usable for debugging purposes.

Tamas / Mateusz,

I happily let you tune this as you see fit

Even

--
Spatialys - Geospatial professional services
http://www.spatialys.com

Mateusz Loskot

2017-10-17 07:59:00 UTC

Permalink

Tamas / Mateusz,

I happily let you tune this as you see fit

I don't want to interfere with Tamas' requirements, but I think simple and
clean from yhr library client point is to dance in Rome like Romans do.
That is stick to canonical build MSVC setup for both release and debug
configurations.
If Tamas needs something different, OK.

FYI, I build GDAL with SDKs provided exclusively in form of optimised
binaries and I haven't encountered any issues, yet.

Mat

Carl Godkin

2017-09-27 22:08:28 UTC

Permalink

Hi Even,

Since you ask, I can tell you that changing "/MD" to "/MDd" is one of just
two remaining edits that I have to make to nmake.opt before I build a new
version of GDAL. We do most of our development on Windows and port to
Linux so I think we do the opposite of how it sounds like you work.

So we'd prefer /MDd.

(By the way, the other item in nmake.opt that I can't override with an
EXT_NMAKE_OPT
file is that we don't want to build with

ODBC_SUPPORTED = 1

If that were changed to use the !IFNDEF ODBC_SUPPORTED then I wouldn't have
to edit nmake.opt at all which would be very handy.)

Thanks very much,

carl

Mateusz Loskot

2017-09-27 22:31:51 UTC

Permalink

Hi Even,

Since you ask, I can tell you that changing "/MD" to "/MDd" is one of just
two remaining edits that I have to make to nmake.opt before I build a new
version of GDAL.

Yup, me too and I think most GDAL as library users on Windows do it as
well.

(By the way, the other item in nmake.opt that I can't override with an
EXT_NMAKE_OPT
file is that we don't want to build with

ODBC_SUPPORTED = 1

If that were changed to use the !IFNDEF ODBC_SUPPORTED then I wouldn't have
to edit nmake.opt at all which would be very handy.)

Then, why not submit a patch or pull request. I think it's good idea.

Mateusz