Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FPyLLL Strategies inside SageMath binaries #28664

Closed
malb opened this issue Oct 28, 2019 · 20 comments
Closed

FPyLLL Strategies inside SageMath binaries #28664

malb opened this issue Oct 28, 2019 · 20 comments

Comments

@malb
Copy link
Member

malb commented Oct 28, 2019

On Debian 8, after downloading Sage 8.8 as follows:

$ curl -O https://www.mirrorservice.org/sites/www.sagemath.org/linux/64bit/sage-8.8-Debian_GNU_Linux_8-x86_64.tar.bz2
$ tar xf sage-8.8-Debian_GNU_Linux_8-x86_64.tar.bz2

and launching Sage as follows:

$ ./SageMath/sage

running the following commands:

sage: from fpylll import *
sage: BKZ.DEFAULT_STRATEGY

gives an incorrect result:

'/.../local/share/fplll/strategieparse error - unpreprocessing_blpruning_parameteError: gptr == nullpoints/default.json'

The output should be a filename but it's not.
A build from source doesn't have that problem.

Reported upstream:

Upstream: Fixed upstream, in a later stable release.

CC: @isuruf @malb @slel @vbraun @dimpase

Component: packages: standard

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/28664

@malb malb added this to the sage-9.0 milestone Oct 28, 2019
@malb
Copy link
Member Author

malb commented Nov 14, 2019

comment:1

Seems a precompiled Sage binary also cannot load strategy files stored elsewhere, so something is really broken there:

┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 8.9, Release Date: 2019-09-29                     │
│ Using Python 2.7.15. Type "help()" for help.                       │
└────────────────────────────────────────────────────────────────────┘
sage: from fpylll import *
sage: BKZ.DEFAULT_STRATEGY_PATH
'/bulk/home/malb/software/SageMath/local/share/fplll/strategieparse error - unpreprocessing_blpruning_parameteError: gptr == nulls'
sage: BKZ.DEFAULT_STRATEGY
'/bulk/home/malb/software/SageMath/local/share/fplll/strategieparse error - unpreprocessing_blpruning_parameteError: gptr == nulls/default.json'
sage: print load_strategies_json("/home/malb/projects/lattices/fplll/strategies/default.json")[60]
Strategy< 60, (), 1.00-1.00>

The above should print Strategy< 60, (40), 0.29-0.50>

See fplll/fpylll#159

Any ideas?

@isuruf
Copy link
Member

isuruf commented Nov 21, 2019

comment:2

This looks like a bug in binary-pkg which looks for \0 character at the end of the path, but the path being stored in a C++ string, the NULL character does not exist at the end.

@isuruf
Copy link
Member

isuruf commented Nov 21, 2019

comment:3

Adding -std=c++11 to CXXFLAGS forces the C++ compiler to enforce C++11 which enforces that std::string terminate with \0 in the internal buffer.

From my limited testing, this fixes the issue.

@malb
Copy link
Member Author

malb commented Nov 23, 2019

comment:4

Oh, thanks!

I'm not clear though on where -std=c++11 is missing, FPLLL is currently being compiled with:

AX_CXX_COMPILE_STDCXX([11],[noext],[mandatory])

which should set -std=c++11. But from my local testing it doesn't seem to (?)

@isuruf
Copy link
Member

isuruf commented Nov 23, 2019

comment:5

File m4/ax_cxx_compile_stdcxx.m4 in fplll repo is old. Updating to the latest version fixes the issue.

@isuruf
Copy link
Member

isuruf commented Nov 23, 2019

comment:6

@malb, the updated macro at https://github.com/fplll/fplll/pull/394/files forces the -std=c++11. New versions of gcc support some C++11 features without the flag, but does not enforce some C++11 features like \0 at the end of the string buffer. This default checking is removed in https://github.com/fplll/fplll/pull/394/files#diff-38b7f4802a0916ba0fab7e320746bfdbL62-L69

@malb
Copy link
Member Author

malb commented Nov 24, 2019

Changed upstream from Reported upstream. Developers deny it's a bug. to Reported upstream. Developers acknowledge bug.

@malb
Copy link
Member Author

malb commented Nov 24, 2019

comment:7

Thank you! I'll wait for the fallout of the 5.3.0 release for a bit and then will cut a new release 5.3.1 with that fix in.

@malb
Copy link
Member Author

malb commented Dec 5, 2019

Changed upstream from Reported upstream. Developers acknowledge bug. to Fixed upstream, but not in a stable release.

@malb
Copy link
Member Author

malb commented Dec 14, 2019

comment:9

Fixed in FPLLL 5.3.1

@malb
Copy link
Member Author

malb commented Dec 14, 2019

Changed upstream from Fixed upstream, but not in a stable release. to Fixed upstream, in a later stable release.

@malb
Copy link
Member Author

malb commented Dec 15, 2019

comment:10

See #28886 for updates of FP(y)LLL which should fix this issue.

@embray
Copy link
Contributor

embray commented Jan 6, 2020

comment:11

Ticket retargeted after milestone closed

@embray embray modified the milestones: sage-9.0, sage-9.1 Jan 6, 2020
@mkoeppe
Copy link
Member

mkoeppe commented May 1, 2020

comment:12

Moving tickets to milestone sage-9.2 based on a review of last modification date, branch status, and severity.

@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 May 1, 2020
@slel

This comment has been minimized.

@slel
Copy link
Member

slel commented Sep 16, 2020

comment:13

On Debian 10 buster, after downloading Sage 9.1 as follows:

$ SAGE_VERSION=9.1
$ DEBIAN_VERSION=`source /etc/os-release ; echo $VERSION_ID`
$ MIRROR=https://www.mirrorservice.org/sites/www.sagemath.org/linux/64bit
$ TARBALL=sage-$SAGE_VERSION-Debian_GNU_Linux_$DEBIAN_VERSION-x86_64.tar.bz2
$ curl -O $MIRROR/$TARBALL
$ tar xf $TARBALL

and launching Sage as follows:

$ ./SageMath/sage -q

running the following two commands:

sage: from fpylll import *
sage: BKZ.DEFAULT_STRATEGY

still gives the same incorrect result:

'/.../local/share/fplll/strategieparse error - unpreprocessing_blpruning_parameteError: gptr == nullpointer.'

while with Sage built from source on the same machine:

sage: from fpylll import *
sage: BKZ.DEFAULT_STRATEGY
b'/.../local/share/fplll/strategies/default.json'

So even after #28886 Sage built from source gets this right
while Sage binaries don't.

@slel
Copy link
Member

slel commented Sep 16, 2020

comment:14

What causes this difference between the buildbot-produced binaries
and Sage built from source?

@isuruf
Copy link
Member

isuruf commented Sep 16, 2020

comment:15

Is fplll built with C++11 in the Sage 9.1 binaries?

@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Oct 24, 2020
@mkoeppe
Copy link
Member

mkoeppe commented May 10, 2021

comment:17

Moving to 9.4, as 9.3 has been released.

@mkoeppe mkoeppe removed this from the sage-9.3 milestone May 10, 2021
@mkoeppe mkoeppe added this to the sage-9.4 milestone May 10, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Aug 22, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Jan 12, 2022
@mkoeppe mkoeppe removed this from the sage-9.6 milestone Feb 9, 2022
@dimpase
Copy link
Member

dimpase commented Feb 9, 2022

Reviewer: Dima Pasechnik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants